From fc05be5b9a03a81abbf68a68bc2d69476032f05b Mon Sep 17 00:00:00 2001 From: Ammar Date: Wed, 29 Oct 2025 17:59:52 +0000 Subject: [PATCH 01/11] =?UTF-8?q?=F0=9F=A4=96=20fix:=20expand=20tilde=20in?= =?UTF-8?q?=20project=20paths=20and=20validate=20existence?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When users input project paths with tilde (~), they were not being expanded correctly, causing all project operations to fail. This fix implements automatic tilde expansion and path validation. Changes: - Add pathUtils module with expandTilde() and validateProjectPath() - Update PROJECT_CREATE handler to validate and normalize paths - Update PROJECT_LIST_BRANCHES handler to handle tilde paths - Prevent adding non-existent paths as projects - Reject file paths (only directories are valid projects) - Normalize paths with .. to avoid duplicate entries Testing: - Unit tests for expandTilde() and validateProjectPath() - Integration tests for PROJECT_CREATE IPC handler covering: * Tilde expansion in project paths * Rejection of non-existent paths * Rejection of file paths (non-directories) * Path normalization with .. segments * Duplicate detection with normalized paths All tests pass. Linter and type checker satisfied. _Generated with `cmux`_ --- src/services/ipcMain.ts | 29 ++++-- src/utils/pathUtils.test.ts | 104 +++++++++++++++++++ src/utils/pathUtils.ts | 86 ++++++++++++++++ tests/ipcMain/projectCreate.test.ts | 148 ++++++++++++++++++++++++++++ 4 files changed, 361 insertions(+), 6 deletions(-) create mode 100644 src/utils/pathUtils.test.ts create mode 100644 src/utils/pathUtils.ts create mode 100644 tests/ipcMain/projectCreate.test.ts diff --git a/src/services/ipcMain.ts b/src/services/ipcMain.ts index 1a195fe96..64c56468c 100644 --- a/src/services/ipcMain.ts +++ b/src/services/ipcMain.ts @@ -26,6 +26,7 @@ import { BashExecutionService } from "@/services/bashExecutionService"; import { InitStateManager } from "@/services/initStateManager"; import { createRuntime } from "@/runtime/runtimeFactory"; import type { RuntimeConfig } from "@/types/runtime"; +import { validateProjectPath } from "@/utils/pathUtils"; /** * IpcMain - Manages all IPC handlers and service coordination * @@ -1181,10 +1182,19 @@ export class IpcMain { private registerProjectHandlers(ipcMain: ElectronIpcMain): void { ipcMain.handle(IPC_CHANNELS.PROJECT_CREATE, (_event, projectPath: string) => { try { + // Validate and expand path (handles tilde, checks existence and directory status) + const validation = validateProjectPath(projectPath); + if (!validation.valid) { + return Err(validation.error ?? "Invalid project path"); + } + + // Use the expanded/normalized path + const normalizedPath = validation.expandedPath!; + const config = this.config.loadConfigOrDefault(); - // Check if project already exists - if (config.projects.has(projectPath)) { + // Check if project already exists (using normalized path) + if (config.projects.has(normalizedPath)) { return Err("Project already exists"); } @@ -1193,8 +1203,8 @@ export class IpcMain { workspaces: [], }; - // Add to config - config.projects.set(projectPath, projectConfig); + // Add to config with normalized path + config.projects.set(normalizedPath, projectConfig); this.config.saveConfig(config); return Ok(projectConfig); @@ -1256,8 +1266,15 @@ export class IpcMain { } try { - const branches = await listLocalBranches(projectPath); - const recommendedTrunk = await detectDefaultTrunkBranch(projectPath, branches); + // Validate and expand path (handles tilde) + const validation = validateProjectPath(projectPath); + if (!validation.valid) { + throw new Error(validation.error ?? "Invalid project path"); + } + + const normalizedPath = validation.expandedPath!; + const branches = await listLocalBranches(normalizedPath); + const recommendedTrunk = await detectDefaultTrunkBranch(normalizedPath, branches); return { branches, recommendedTrunk }; } catch (error) { log.error("Failed to list branches:", error); diff --git a/src/utils/pathUtils.test.ts b/src/utils/pathUtils.test.ts new file mode 100644 index 000000000..e49503ad0 --- /dev/null +++ b/src/utils/pathUtils.test.ts @@ -0,0 +1,104 @@ +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; +import { expandTilde, validateProjectPath } from "./pathUtils"; + +describe("pathUtils", () => { + describe("expandTilde", () => { + it("should expand ~ to home directory", () => { + const result = expandTilde("~/Documents"); + const expected = path.join(os.homedir(), "Documents"); + expect(result).toBe(expected); + }); + + it("should expand ~/ to home directory with trailing path", () => { + const result = expandTilde("~/Projects/my-app"); + const expected = path.join(os.homedir(), "Projects", "my-app"); + expect(result).toBe(expected); + }); + + it("should return path unchanged if it doesn't start with ~", () => { + const testPath = "/absolute/path/to/project"; + const result = expandTilde(testPath); + expect(result).toBe(testPath); + }); + + it("should handle ~ alone (home directory)", () => { + const result = expandTilde("~"); + expect(result).toBe(os.homedir()); + }); + + it("should handle relative paths without tilde", () => { + const relativePath = "relative/path"; + const result = expandTilde(relativePath); + expect(result).toBe(relativePath); + }); + + it("should handle empty string", () => { + const result = expandTilde(""); + expect(result).toBe(""); + }); + }); + + describe("validateProjectPath", () => { + let tempDir: string; + + beforeEach(() => { + // Create a temporary directory for testing + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "cmux-path-test-")); + }); + + afterEach(() => { + // Clean up temporary directory + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + it("should return success for existing directory", () => { + const result = validateProjectPath(tempDir); + expect(result.valid).toBe(true); + expect(result.expandedPath).toBe(tempDir); + expect(result.error).toBeUndefined(); + }); + + it("should expand tilde and validate", () => { + // Use a path that we know exists in home directory + const homeDir = os.homedir(); + const result = validateProjectPath("~"); + expect(result.valid).toBe(true); + expect(result.expandedPath).toBe(homeDir); + expect(result.error).toBeUndefined(); + }); + + it("should return error for non-existent path", () => { + const nonExistentPath = "/this/path/definitely/does/not/exist/cmux-test-12345"; + const result = validateProjectPath(nonExistentPath); + expect(result.valid).toBe(false); + expect(result.error).toContain("does not exist"); + }); + + it("should return error for file path (not directory)", () => { + const filePath = path.join(tempDir, "test-file.txt"); + // eslint-disable-next-line local/no-sync-fs-methods -- Test setup only + fs.writeFileSync(filePath, "test content"); + + const result = validateProjectPath(filePath); + expect(result.valid).toBe(false); + expect(result.error).toContain("not a directory"); + }); + + it("should handle tilde path to non-existent directory", () => { + const nonExistentTildePath = "~/this-directory-should-not-exist-cmux-test-12345"; + const result = validateProjectPath(nonExistentTildePath); + expect(result.valid).toBe(false); + expect(result.error).toContain("does not exist"); + }); + + it("should return normalized absolute path", () => { + const pathWithDots = path.join(tempDir, "..", path.basename(tempDir)); + const result = validateProjectPath(pathWithDots); + expect(result.valid).toBe(true); + expect(result.expandedPath).toBe(tempDir); + }); + }); +}); + diff --git a/src/utils/pathUtils.ts b/src/utils/pathUtils.ts new file mode 100644 index 000000000..85d4a59ca --- /dev/null +++ b/src/utils/pathUtils.ts @@ -0,0 +1,86 @@ +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; + +/** + * Result of path validation + */ +export interface PathValidationResult { + valid: boolean; + expandedPath?: string; + error?: string; +} + +/** + * Expand tilde (~) in paths to the user's home directory + * + * @param inputPath - Path that may contain tilde + * @returns Path with tilde expanded to home directory + * + * @example + * expandTilde("~/Documents") // => "/home/user/Documents" + * expandTilde("~") // => "/home/user" + * expandTilde("/absolute/path") // => "/absolute/path" + */ +export function expandTilde(inputPath: string): string { + if (!inputPath) { + return inputPath; + } + + if (inputPath === "~") { + return os.homedir(); + } + + if (inputPath.startsWith("~/") || inputPath.startsWith("~\\")) { + return path.join(os.homedir(), inputPath.slice(2)); + } + + return inputPath; +} + +/** + * Validate that a project path exists and is a directory + * Automatically expands tilde and normalizes the path + * + * @param inputPath - Path to validate (may contain tilde) + * @returns Validation result with expanded path or error + * + * @example + * validateProjectPath("~/my-project") + * // => { valid: true, expandedPath: "/home/user/my-project" } + * + * validateProjectPath("~/nonexistent") + * // => { valid: false, error: "Path does not exist: /home/user/nonexistent" } + */ +export function validateProjectPath(inputPath: string): PathValidationResult { + // Expand tilde if present + const expandedPath = expandTilde(inputPath); + + // Normalize to resolve any .. or . in the path + const normalizedPath = path.normalize(expandedPath); + + // Check if path exists + // eslint-disable-next-line local/no-sync-fs-methods -- Synchronous validation required for IPC handler + if (!fs.existsSync(normalizedPath)) { + return { + valid: false, + error: `Path does not exist: ${normalizedPath}`, + }; + } + + // Check if it's a directory + // eslint-disable-next-line local/no-sync-fs-methods -- Synchronous validation required for IPC handler + const stats = fs.statSync(normalizedPath); + if (!stats.isDirectory()) { + return { + valid: false, + error: `Path is not a directory: ${normalizedPath}`, + }; + } + + return { + valid: true, + expandedPath: normalizedPath, + }; +} + diff --git a/tests/ipcMain/projectCreate.test.ts b/tests/ipcMain/projectCreate.test.ts new file mode 100644 index 000000000..cc03ab31b --- /dev/null +++ b/tests/ipcMain/projectCreate.test.ts @@ -0,0 +1,148 @@ +/** + * Integration tests for PROJECT_CREATE IPC handler + * + * Tests: + * - Tilde expansion in project paths + * - Path validation (existence, directory check) + * - Prevention of adding non-existent projects + */ + +import * as fs from "fs/promises"; +import * as path from "path"; +import * as os from "os"; +import { shouldRunIntegrationTests, createTestEnvironment, cleanupTestEnvironment } from "./setup"; +import type { TestEnvironment } from "./setup"; +import { IPC_CHANNELS } from "../../src/constants/ipc-constants"; + +const describeIntegration = shouldRunIntegrationTests() ? describe : describe.skip; + +describeIntegration("PROJECT_CREATE IPC Handler", () => { + test.concurrent("should expand tilde in project path and create project", async () => { + const env = await createTestEnvironment(); + const tempProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), "cmux-project-test-")); + // Create a test directory in home directory + const testDirName = `cmux-test-tilde-${Date.now()}`; + const homeProjectPath = path.join(os.homedir(), testDirName); + await fs.mkdir(homeProjectPath, { recursive: true }); + + try { + // Try to create project with tilde path + const tildeProjectPath = `~/${testDirName}`; + const result = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_CREATE, tildeProjectPath); + + // Should succeed + expect(result.success).toBe(true); + + // Verify the project was added with expanded path (not tilde path) + const projectsList = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_LIST); + const projectPaths = projectsList.map((p: [string, unknown]) => p[0]); + + // Should contain the expanded path + expect(projectPaths).toContain(homeProjectPath); + // Should NOT contain the tilde path + expect(projectPaths).not.toContain(tildeProjectPath); + } finally { + // Clean up test directory + await fs.rm(homeProjectPath, { recursive: true, force: true }); + await cleanupTestEnvironment(env); + await fs.rm(tempProjectDir, { recursive: true, force: true }); + } + }); + + test.concurrent("should reject non-existent project path", async () => { + const env = await createTestEnvironment(); + const tempProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), "cmux-project-test-")); + const nonExistentPath = "/this/path/definitely/does/not/exist/cmux-test-12345"; + const result = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_CREATE, nonExistentPath); + + expect(result.success).toBe(false); + expect(result.error).toContain("does not exist"); + + await cleanupTestEnvironment(env); + await fs.rm(tempProjectDir, { recursive: true, force: true }); + }); + + test.concurrent("should reject non-existent tilde path", async () => { + const env = await createTestEnvironment(); + const tempProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), "cmux-project-test-")); + const nonExistentTildePath = "~/this-directory-should-not-exist-cmux-test-12345"; + const result = await env.mockIpcRenderer.invoke( + IPC_CHANNELS.PROJECT_CREATE, + nonExistentTildePath + ); + + expect(result.success).toBe(false); + expect(result.error).toContain("does not exist"); + + await cleanupTestEnvironment(env); + await fs.rm(tempProjectDir, { recursive: true, force: true }); + }); + + test.concurrent("should reject file path (not a directory)", async () => { + const env = await createTestEnvironment(); + const tempProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), "cmux-project-test-")); + const testFile = path.join(tempProjectDir, "test-file.txt"); + await fs.writeFile(testFile, "test content"); + + const result = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_CREATE, testFile); + + expect(result.success).toBe(false); + expect(result.error).toContain("not a directory"); + + await cleanupTestEnvironment(env); + await fs.rm(tempProjectDir, { recursive: true, force: true }); + }); + + test.concurrent("should accept valid absolute path", async () => { + const env = await createTestEnvironment(); + const tempProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), "cmux-project-test-")); + const result = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_CREATE, tempProjectDir); + + expect(result.success).toBe(true); + + // Verify project was added + const projectsList = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_LIST); + const projectPaths = projectsList.map((p: [string, unknown]) => p[0]); + expect(projectPaths).toContain(tempProjectDir); + + await cleanupTestEnvironment(env); + await fs.rm(tempProjectDir, { recursive: true, force: true }); + }); + + test.concurrent("should normalize paths with .. in them", async () => { + const env = await createTestEnvironment(); + const tempProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), "cmux-project-test-")); + // Create a path with .. that resolves to tempProjectDir + const pathWithDots = path.join(tempProjectDir, "..", path.basename(tempProjectDir)); + const result = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_CREATE, pathWithDots); + + expect(result.success).toBe(true); + + // Verify project was added with normalized path + const projectsList = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_LIST); + const projectPaths = projectsList.map((p: [string, unknown]) => p[0]); + expect(projectPaths).toContain(tempProjectDir); + + await cleanupTestEnvironment(env); + await fs.rm(tempProjectDir, { recursive: true, force: true }); + }); + + test.concurrent("should reject duplicate projects (same expanded path)", async () => { + const env = await createTestEnvironment(); + const tempProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), "cmux-project-test-")); + // Create first project + const result1 = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_CREATE, tempProjectDir); + expect(result1.success).toBe(true); + + // Try to create the same project with a path that has .. + const pathWithDots = path.join(tempProjectDir, "..", path.basename(tempProjectDir)); + const result2 = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_CREATE, pathWithDots); + + expect(result2.success).toBe(false); + expect(result2.error).toContain("already exists"); + + await cleanupTestEnvironment(env); + await fs.rm(tempProjectDir, { recursive: true, force: true }); + }); +}); + From aa8502d9c156a12170a106ab8b4d43fc9e339e6b Mon Sep 17 00:00:00 2001 From: Ammar Date: Thu, 30 Oct 2025 14:37:21 +0000 Subject: [PATCH 02/11] fix: format with prettier --- src/utils/pathUtils.test.ts | 3 +-- src/utils/pathUtils.ts | 13 ++++++------- tests/ipcMain/projectCreate.test.ts | 10 ++++++---- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/utils/pathUtils.test.ts b/src/utils/pathUtils.test.ts index e49503ad0..96cfdfa03 100644 --- a/src/utils/pathUtils.test.ts +++ b/src/utils/pathUtils.test.ts @@ -80,7 +80,7 @@ describe("pathUtils", () => { const filePath = path.join(tempDir, "test-file.txt"); // eslint-disable-next-line local/no-sync-fs-methods -- Test setup only fs.writeFileSync(filePath, "test content"); - + const result = validateProjectPath(filePath); expect(result.valid).toBe(false); expect(result.error).toContain("not a directory"); @@ -101,4 +101,3 @@ describe("pathUtils", () => { }); }); }); - diff --git a/src/utils/pathUtils.ts b/src/utils/pathUtils.ts index 85d4a59ca..40461d71e 100644 --- a/src/utils/pathUtils.ts +++ b/src/utils/pathUtils.ts @@ -13,10 +13,10 @@ export interface PathValidationResult { /** * Expand tilde (~) in paths to the user's home directory - * + * * @param inputPath - Path that may contain tilde * @returns Path with tilde expanded to home directory - * + * * @example * expandTilde("~/Documents") // => "/home/user/Documents" * expandTilde("~") // => "/home/user" @@ -41,21 +41,21 @@ export function expandTilde(inputPath: string): string { /** * Validate that a project path exists and is a directory * Automatically expands tilde and normalizes the path - * + * * @param inputPath - Path to validate (may contain tilde) * @returns Validation result with expanded path or error - * + * * @example * validateProjectPath("~/my-project") * // => { valid: true, expandedPath: "/home/user/my-project" } - * + * * validateProjectPath("~/nonexistent") * // => { valid: false, error: "Path does not exist: /home/user/nonexistent" } */ export function validateProjectPath(inputPath: string): PathValidationResult { // Expand tilde if present const expandedPath = expandTilde(inputPath); - + // Normalize to resolve any .. or . in the path const normalizedPath = path.normalize(expandedPath); @@ -83,4 +83,3 @@ export function validateProjectPath(inputPath: string): PathValidationResult { expandedPath: normalizedPath, }; } - diff --git a/tests/ipcMain/projectCreate.test.ts b/tests/ipcMain/projectCreate.test.ts index cc03ab31b..d5df34f90 100644 --- a/tests/ipcMain/projectCreate.test.ts +++ b/tests/ipcMain/projectCreate.test.ts @@ -1,6 +1,6 @@ /** * Integration tests for PROJECT_CREATE IPC handler - * + * * Tests: * - Tilde expansion in project paths * - Path validation (existence, directory check) @@ -28,7 +28,10 @@ describeIntegration("PROJECT_CREATE IPC Handler", () => { try { // Try to create project with tilde path const tildeProjectPath = `~/${testDirName}`; - const result = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_CREATE, tildeProjectPath); + const result = await env.mockIpcRenderer.invoke( + IPC_CHANNELS.PROJECT_CREATE, + tildeProjectPath + ); // Should succeed expect(result.success).toBe(true); @@ -36,7 +39,7 @@ describeIntegration("PROJECT_CREATE IPC Handler", () => { // Verify the project was added with expanded path (not tilde path) const projectsList = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_LIST); const projectPaths = projectsList.map((p: [string, unknown]) => p[0]); - + // Should contain the expanded path expect(projectPaths).toContain(homeProjectPath); // Should NOT contain the tilde path @@ -145,4 +148,3 @@ describeIntegration("PROJECT_CREATE IPC Handler", () => { await fs.rm(tempProjectDir, { recursive: true, force: true }); }); }); - From 3bd614f40ba57a65b8d917f21c9a3d5d2cc6af09 Mon Sep 17 00:00:00 2001 From: Ammar Date: Thu, 30 Oct 2025 14:46:18 +0000 Subject: [PATCH 03/11] feat: require .git and fix duplicate detection - Add .git directory validation to prevent non-git paths - Return normalized path from PROJECT_CREATE for frontend use - Fix duplicate detection using backend-normalized paths - Add proper error display with alert() in frontend - Update integration tests for new response format Fixes issues: - Projects showing in UI even when creation failed - Duplicate projects appearing due to path normalization mismatch - Missing git repository validation - Silent errors not shown to users --- src/App.stories.tsx | 6 ++++- src/hooks/useProjectManagement.ts | 22 +++++++++++----- src/services/ipcMain.ts | 3 ++- src/types/ipc.ts | 4 ++- src/utils/pathUtils.test.ts | 40 +++++++++++++++++++++++++---- src/utils/pathUtils.ts | 15 ++++++++++- tests/ipcMain/projectCreate.test.ts | 27 +++++++++++++++++++ 7 files changed, 102 insertions(+), 15 deletions(-) diff --git a/src/App.stories.tsx b/src/App.stories.tsx index 9cfcdce36..dc9d3f939 100644 --- a/src/App.stories.tsx +++ b/src/App.stories.tsx @@ -64,7 +64,11 @@ function setupMockAPI(options: { }, projects: { list: () => Promise.resolve(Array.from(mockProjects.entries())), - create: () => Promise.resolve({ success: true, data: { workspaces: [] } }), + create: () => + Promise.resolve({ + success: true, + data: { projectConfig: { workspaces: [] }, normalizedPath: "/mock/project/path" }, + }), remove: () => Promise.resolve({ success: true, data: undefined }), listBranches: () => Promise.resolve({ diff --git a/src/hooks/useProjectManagement.ts b/src/hooks/useProjectManagement.ts index 3fd2ccffd..96f2479b0 100644 --- a/src/hooks/useProjectManagement.ts +++ b/src/hooks/useProjectManagement.ts @@ -31,20 +31,30 @@ export function useProjectManagement() { const selectedPath = await window.api.dialog.selectDirectory(); if (!selectedPath) return; - if (projects.has(selectedPath)) { - console.log("Project already exists:", selectedPath); - return; - } - const result = await window.api.projects.create(selectedPath); if (result.success) { + // Use the normalized path returned from backend + const { normalizedPath, projectConfig } = result.data; + + // Check if already exists using normalized path + if (projects.has(normalizedPath)) { + console.log("Project already exists:", normalizedPath); + alert("This project has already been added."); + return; + } + const newProjects = new Map(projects); - newProjects.set(selectedPath, result.data); + newProjects.set(normalizedPath, projectConfig); setProjects(newProjects); } else { + // Show error to user + const errorMessage = typeof result.error === "string" ? result.error : "Failed to add project"; + alert(errorMessage); console.error("Failed to create project:", result.error); } } catch (error) { + const errorMessage = error instanceof Error ? error.message : "An unexpected error occurred"; + alert(`Failed to add project: ${errorMessage}`); console.error("Failed to add project:", error); } }, [projects]); diff --git a/src/services/ipcMain.ts b/src/services/ipcMain.ts index 64c56468c..47879631f 100644 --- a/src/services/ipcMain.ts +++ b/src/services/ipcMain.ts @@ -1207,7 +1207,8 @@ export class IpcMain { config.projects.set(normalizedPath, projectConfig); this.config.saveConfig(config); - return Ok(projectConfig); + // Return both the config and the normalized path so frontend can use it + return Ok({ projectConfig, normalizedPath }); } catch (error) { const message = error instanceof Error ? error.message : String(error); return Err(`Failed to create project: ${message}`); diff --git a/src/types/ipc.ts b/src/types/ipc.ts index 7ae90ee34..c4cf143eb 100644 --- a/src/types/ipc.ts +++ b/src/types/ipc.ts @@ -212,7 +212,9 @@ export interface IPCApi { list(): Promise; }; projects: { - create(projectPath: string): Promise>; + create(projectPath: string): Promise< + Result<{ projectConfig: ProjectConfig; normalizedPath: string }, string> + >; remove(projectPath: string): Promise>; list(): Promise>; listBranches(projectPath: string): Promise; diff --git a/src/utils/pathUtils.test.ts b/src/utils/pathUtils.test.ts index 96cfdfa03..3d6e8fbff 100644 --- a/src/utils/pathUtils.test.ts +++ b/src/utils/pathUtils.test.ts @@ -53,7 +53,10 @@ describe("pathUtils", () => { fs.rmSync(tempDir, { recursive: true, force: true }); }); - it("should return success for existing directory", () => { + it("should return success for existing git directory", () => { + // Create .git directory + // eslint-disable-next-line local/no-sync-fs-methods -- Test setup only + fs.mkdirSync(path.join(tempDir, ".git")); const result = validateProjectPath(tempDir); expect(result.valid).toBe(true); expect(result.expandedPath).toBe(tempDir); @@ -61,12 +64,20 @@ describe("pathUtils", () => { }); it("should expand tilde and validate", () => { - // Use a path that we know exists in home directory - const homeDir = os.homedir(); - const result = validateProjectPath("~"); + // Create a test directory in home with .git + const testDir = path.join(os.homedir(), `cmux-test-git-${Date.now()}`); + // eslint-disable-next-line local/no-sync-fs-methods -- Test setup only + fs.mkdirSync(testDir, { recursive: true }); + // eslint-disable-next-line local/no-sync-fs-methods -- Test setup only + fs.mkdirSync(path.join(testDir, ".git")); + + const result = validateProjectPath(`~/${path.basename(testDir)}`); expect(result.valid).toBe(true); - expect(result.expandedPath).toBe(homeDir); + expect(result.expandedPath).toBe(testDir); expect(result.error).toBeUndefined(); + + // Cleanup + fs.rmSync(testDir, { recursive: true, force: true }); }); it("should return error for non-existent path", () => { @@ -95,9 +106,28 @@ describe("pathUtils", () => { it("should return normalized absolute path", () => { const pathWithDots = path.join(tempDir, "..", path.basename(tempDir)); + // Add .git directory for validation + // eslint-disable-next-line local/no-sync-fs-methods -- Test setup only + fs.mkdirSync(path.join(tempDir, ".git")); const result = validateProjectPath(pathWithDots); expect(result.valid).toBe(true); expect(result.expandedPath).toBe(tempDir); }); + + it("should reject directory without .git", () => { + const result = validateProjectPath(tempDir); + expect(result.valid).toBe(false); + expect(result.error).toContain("Not a git repository"); + }); + + it("should accept directory with .git", () => { + const gitDir = path.join(tempDir, ".git"); + // eslint-disable-next-line local/no-sync-fs-methods -- Test setup only + fs.mkdirSync(gitDir); + + const result = validateProjectPath(tempDir); + expect(result.valid).toBe(true); + expect(result.expandedPath).toBe(tempDir); + }); }); }); diff --git a/src/utils/pathUtils.ts b/src/utils/pathUtils.ts index 40461d71e..fc505ebcd 100644 --- a/src/utils/pathUtils.ts +++ b/src/utils/pathUtils.ts @@ -39,7 +39,7 @@ export function expandTilde(inputPath: string): string { } /** - * Validate that a project path exists and is a directory + * Validate that a project path exists, is a directory, and is a git repository * Automatically expands tilde and normalizes the path * * @param inputPath - Path to validate (may contain tilde) @@ -51,6 +51,9 @@ export function expandTilde(inputPath: string): string { * * validateProjectPath("~/nonexistent") * // => { valid: false, error: "Path does not exist: /home/user/nonexistent" } + * + * validateProjectPath("~/not-a-git-repo") + * // => { valid: false, error: "Not a git repository: /home/user/not-a-git-repo" } */ export function validateProjectPath(inputPath: string): PathValidationResult { // Expand tilde if present @@ -78,6 +81,16 @@ export function validateProjectPath(inputPath: string): PathValidationResult { }; } + // Check if it's a git repository + const gitPath = path.join(normalizedPath, ".git"); + // eslint-disable-next-line local/no-sync-fs-methods -- Synchronous validation required for IPC handler + if (!fs.existsSync(gitPath)) { + return { + valid: false, + error: `Not a git repository: ${normalizedPath}`, + }; + } + return { valid: true, expandedPath: normalizedPath, diff --git a/tests/ipcMain/projectCreate.test.ts b/tests/ipcMain/projectCreate.test.ts index d5df34f90..c81557868 100644 --- a/tests/ipcMain/projectCreate.test.ts +++ b/tests/ipcMain/projectCreate.test.ts @@ -24,6 +24,8 @@ describeIntegration("PROJECT_CREATE IPC Handler", () => { const testDirName = `cmux-test-tilde-${Date.now()}`; const homeProjectPath = path.join(os.homedir(), testDirName); await fs.mkdir(homeProjectPath, { recursive: true }); + // Create .git directory to make it a valid git repo + await fs.mkdir(path.join(homeProjectPath, ".git")); try { // Try to create project with tilde path @@ -35,6 +37,7 @@ describeIntegration("PROJECT_CREATE IPC Handler", () => { // Should succeed expect(result.success).toBe(true); + expect(result.data.normalizedPath).toBe(homeProjectPath); // Verify the project was added with expanded path (not tilde path) const projectsList = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_LIST); @@ -96,12 +99,29 @@ describeIntegration("PROJECT_CREATE IPC Handler", () => { await fs.rm(tempProjectDir, { recursive: true, force: true }); }); + test.concurrent("should reject directory without .git", async () => { + const env = await createTestEnvironment(); + const tempProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), "cmux-project-test-")); + + const result = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_CREATE, tempProjectDir); + + expect(result.success).toBe(false); + expect(result.error).toContain("Not a git repository"); + + await cleanupTestEnvironment(env); + await fs.rm(tempProjectDir, { recursive: true, force: true }); + }); + test.concurrent("should accept valid absolute path", async () => { const env = await createTestEnvironment(); const tempProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), "cmux-project-test-")); + // Create .git directory to make it a valid git repo + await fs.mkdir(path.join(tempProjectDir, ".git")); + const result = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_CREATE, tempProjectDir); expect(result.success).toBe(true); + expect(result.data.normalizedPath).toBe(tempProjectDir); // Verify project was added const projectsList = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_LIST); @@ -115,11 +135,15 @@ describeIntegration("PROJECT_CREATE IPC Handler", () => { test.concurrent("should normalize paths with .. in them", async () => { const env = await createTestEnvironment(); const tempProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), "cmux-project-test-")); + // Create .git directory to make it a valid git repo + await fs.mkdir(path.join(tempProjectDir, ".git")); + // Create a path with .. that resolves to tempProjectDir const pathWithDots = path.join(tempProjectDir, "..", path.basename(tempProjectDir)); const result = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_CREATE, pathWithDots); expect(result.success).toBe(true); + expect(result.data.normalizedPath).toBe(tempProjectDir); // Verify project was added with normalized path const projectsList = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_LIST); @@ -133,6 +157,9 @@ describeIntegration("PROJECT_CREATE IPC Handler", () => { test.concurrent("should reject duplicate projects (same expanded path)", async () => { const env = await createTestEnvironment(); const tempProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), "cmux-project-test-")); + // Create .git directory to make it a valid git repo + await fs.mkdir(path.join(tempProjectDir, ".git")); + // Create first project const result1 = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_CREATE, tempProjectDir); expect(result1.success).toBe(true); From 1e9678ea8efd7b180d626d6921d3715794525e49 Mon Sep 17 00:00:00 2001 From: Ammar Date: Thu, 30 Oct 2025 14:48:23 +0000 Subject: [PATCH 04/11] fix: format with prettier --- src/hooks/useProjectManagement.ts | 3 ++- src/types/ipc.ts | 6 +++--- src/utils/pathUtils.test.ts | 4 ++-- tests/ipcMain/projectCreate.test.ts | 6 +++--- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/hooks/useProjectManagement.ts b/src/hooks/useProjectManagement.ts index 96f2479b0..672e890f7 100644 --- a/src/hooks/useProjectManagement.ts +++ b/src/hooks/useProjectManagement.ts @@ -48,7 +48,8 @@ export function useProjectManagement() { setProjects(newProjects); } else { // Show error to user - const errorMessage = typeof result.error === "string" ? result.error : "Failed to add project"; + const errorMessage = + typeof result.error === "string" ? result.error : "Failed to add project"; alert(errorMessage); console.error("Failed to create project:", result.error); } diff --git a/src/types/ipc.ts b/src/types/ipc.ts index c4cf143eb..cf7fd329f 100644 --- a/src/types/ipc.ts +++ b/src/types/ipc.ts @@ -212,9 +212,9 @@ export interface IPCApi { list(): Promise; }; projects: { - create(projectPath: string): Promise< - Result<{ projectConfig: ProjectConfig; normalizedPath: string }, string> - >; + create( + projectPath: string + ): Promise>; remove(projectPath: string): Promise>; list(): Promise>; listBranches(projectPath: string): Promise; diff --git a/src/utils/pathUtils.test.ts b/src/utils/pathUtils.test.ts index 3d6e8fbff..b9991ab29 100644 --- a/src/utils/pathUtils.test.ts +++ b/src/utils/pathUtils.test.ts @@ -70,12 +70,12 @@ describe("pathUtils", () => { fs.mkdirSync(testDir, { recursive: true }); // eslint-disable-next-line local/no-sync-fs-methods -- Test setup only fs.mkdirSync(path.join(testDir, ".git")); - + const result = validateProjectPath(`~/${path.basename(testDir)}`); expect(result.valid).toBe(true); expect(result.expandedPath).toBe(testDir); expect(result.error).toBeUndefined(); - + // Cleanup fs.rmSync(testDir, { recursive: true, force: true }); }); diff --git a/tests/ipcMain/projectCreate.test.ts b/tests/ipcMain/projectCreate.test.ts index c81557868..89e58d14a 100644 --- a/tests/ipcMain/projectCreate.test.ts +++ b/tests/ipcMain/projectCreate.test.ts @@ -117,7 +117,7 @@ describeIntegration("PROJECT_CREATE IPC Handler", () => { const tempProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), "cmux-project-test-")); // Create .git directory to make it a valid git repo await fs.mkdir(path.join(tempProjectDir, ".git")); - + const result = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_CREATE, tempProjectDir); expect(result.success).toBe(true); @@ -137,7 +137,7 @@ describeIntegration("PROJECT_CREATE IPC Handler", () => { const tempProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), "cmux-project-test-")); // Create .git directory to make it a valid git repo await fs.mkdir(path.join(tempProjectDir, ".git")); - + // Create a path with .. that resolves to tempProjectDir const pathWithDots = path.join(tempProjectDir, "..", path.basename(tempProjectDir)); const result = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_CREATE, pathWithDots); @@ -159,7 +159,7 @@ describeIntegration("PROJECT_CREATE IPC Handler", () => { const tempProjectDir = await fs.mkdtemp(path.join(os.tmpdir(), "cmux-project-test-")); // Create .git directory to make it a valid git repo await fs.mkdir(path.join(tempProjectDir, ".git")); - + // Create first project const result1 = await env.mockIpcRenderer.invoke(IPC_CHANNELS.PROJECT_CREATE, tempProjectDir); expect(result1.success).toBe(true); From 1b22071eb40a72c77f310f34d1a101e39130c968 Mon Sep 17 00:00:00 2001 From: Ammar Date: Thu, 30 Oct 2025 15:02:06 +0000 Subject: [PATCH 05/11] =?UTF-8?q?=F0=9F=A4=96=20refactor:=20replace=20aler?= =?UTF-8?q?t()=20with=20ProjectErrorModal=20for=20project=20errors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace all alert() calls in useProjectManagement with proper error state - Add ProjectErrorModal component to display validation and create errors - Wire error state through AppContext to App component - Clean up unnecessary console.log statements in loadProjects - Add Storybook stories for ProjectErrorModal demonstrating all error types Error flow now properly blocks project creation in both native dialog and web versions without using alert(). Errors are displayed in a modal that must be dismissed before continuing. _Generated with `cmux`_ --- src/App.tsx | 4 ++ src/components/AppLoader.tsx | 2 + src/components/ProjectErrorModal.stories.tsx | 64 ++++++++++++++++++++ src/components/ProjectErrorModal.tsx | 40 ++++++++++++ src/contexts/AppContext.tsx | 2 + src/hooks/useProjectManagement.ts | 22 ++++--- 6 files changed, 125 insertions(+), 9 deletions(-) create mode 100644 src/components/ProjectErrorModal.stories.tsx create mode 100644 src/components/ProjectErrorModal.tsx diff --git a/src/App.tsx b/src/App.tsx index dbed6f47f..5b518ecad 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -6,6 +6,7 @@ import type { FrontendWorkspaceMetadata } from "./types/workspace"; import { LeftSidebar } from "./components/LeftSidebar"; import NewWorkspaceModal from "./components/NewWorkspaceModal"; import { DirectorySelectModal } from "./components/DirectorySelectModal"; +import { ProjectErrorModal } from "./components/ProjectErrorModal"; import { AIView } from "./components/AIView"; import { ErrorBoundary } from "./components/ErrorBoundary"; import { usePersistedState, updatePersistedState } from "./hooks/usePersistedState"; @@ -38,6 +39,8 @@ function AppInner() { projects, addProject, removeProject, + projectError, + clearProjectError, workspaceMetadata, setWorkspaceMetadata, createWorkspace, @@ -695,6 +698,7 @@ function AppInner() { /> )} + ); diff --git a/src/components/AppLoader.tsx b/src/components/AppLoader.tsx index 739eee2c7..b41d124f8 100644 --- a/src/components/AppLoader.tsx +++ b/src/components/AppLoader.tsx @@ -108,6 +108,8 @@ export function AppLoader() { setProjects={projectManagement.setProjects} addProject={projectManagement.addProject} removeProject={projectManagement.removeProject} + projectError={projectManagement.error} + clearProjectError={projectManagement.clearError} workspaceMetadata={workspaceManagement.workspaceMetadata} setWorkspaceMetadata={workspaceManagement.setWorkspaceMetadata} createWorkspace={workspaceManagement.createWorkspace} diff --git a/src/components/ProjectErrorModal.stories.tsx b/src/components/ProjectErrorModal.stories.tsx new file mode 100644 index 000000000..2c7b9fc50 --- /dev/null +++ b/src/components/ProjectErrorModal.stories.tsx @@ -0,0 +1,64 @@ +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { action } from "storybook/actions"; +import { ProjectErrorModal } from "./ProjectErrorModal"; + +const meta = { + title: "Components/ProjectErrorModal", + component: ProjectErrorModal, + parameters: { + layout: "fullscreen", + }, + tags: ["autodocs"], + argTypes: { + error: { + control: "text", + description: "Error message to display (null = modal closed)", + }, + onClose: { + control: false, + action: "onClose", + }, + }, + args: { + onClose: action("close-clicked"), + }, +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +export const NotAGitRepository: Story = { + args: { + error: "Not a git repository: /home/user/my-project", + }, +}; + +export const PathDoesNotExist: Story = { + args: { + error: "Path does not exist: /home/user/nonexistent-project", + }, +}; + +export const PathIsNotDirectory: Story = { + args: { + error: "Path is not a directory: /home/user/file.txt", + }, +}; + +export const ProjectAlreadyExists: Story = { + args: { + error: "This project has already been added.", + }, +}; + +export const GenericError: Story = { + args: { + error: "Failed to add project: An unexpected error occurred", + }, +}; + +export const Closed: Story = { + args: { + error: null, + }, +}; diff --git a/src/components/ProjectErrorModal.tsx b/src/components/ProjectErrorModal.tsx new file mode 100644 index 000000000..ebbef3a5b --- /dev/null +++ b/src/components/ProjectErrorModal.tsx @@ -0,0 +1,40 @@ +import React, { useEffect } from "react"; +import { Modal, ModalActions, PrimaryButton } from "./Modal"; + +interface ProjectErrorModalProps { + error: string | null; + onClose: () => void; +} + +/** + * Modal for displaying project-related errors (add/remove failures, validation errors, etc.). + * Blocks the create flow until dismissed. + */ +export const ProjectErrorModal: React.FC = ({ error, onClose }) => { + // Auto-focus the close button for keyboard accessibility + useEffect(() => { + if (error) { + const handleEscape = (e: KeyboardEvent) => { + if (e.key === "Escape") { + onClose(); + } + }; + window.addEventListener("keydown", handleEscape); + return () => window.removeEventListener("keydown", handleEscape); + } + }, [error, onClose]); + + return ( + +
{error}
+ + OK + +
+ ); +}; diff --git a/src/contexts/AppContext.tsx b/src/contexts/AppContext.tsx index 5663dea1f..92d9d3fef 100644 --- a/src/contexts/AppContext.tsx +++ b/src/contexts/AppContext.tsx @@ -15,6 +15,8 @@ interface AppContextType { setProjects: Dispatch>>; addProject: () => Promise; removeProject: (path: string) => Promise; + projectError: string | null; + clearProjectError: () => void; // Workspaces workspaceMetadata: Map; diff --git a/src/hooks/useProjectManagement.ts b/src/hooks/useProjectManagement.ts index 672e890f7..b54de6b68 100644 --- a/src/hooks/useProjectManagement.ts +++ b/src/hooks/useProjectManagement.ts @@ -6,6 +6,7 @@ import type { ProjectConfig } from "@/config"; */ export function useProjectManagement() { const [projects, setProjects] = useState>(new Map()); + const [error, setError] = useState(null); useEffect(() => { void loadProjects(); @@ -13,12 +14,8 @@ export function useProjectManagement() { const loadProjects = async () => { try { - console.log("Loading projects..."); const projectsList = await window.api.projects.list(); - console.log("Received projects:", projectsList); - const projectsMap = new Map(projectsList); - console.log("Created projects map, size:", projectsMap.size); setProjects(projectsMap); } catch (error) { console.error("Failed to load projects:", error); @@ -27,6 +24,7 @@ export function useProjectManagement() { }; const addProject = useCallback(async () => { + setError(null); try { const selectedPath = await window.api.dialog.selectDirectory(); if (!selectedPath) return; @@ -38,8 +36,7 @@ export function useProjectManagement() { // Check if already exists using normalized path if (projects.has(normalizedPath)) { - console.log("Project already exists:", normalizedPath); - alert("This project has already been added."); + setError("This project has already been added."); return; } @@ -50,18 +47,19 @@ export function useProjectManagement() { // Show error to user const errorMessage = typeof result.error === "string" ? result.error : "Failed to add project"; - alert(errorMessage); + setError(errorMessage); console.error("Failed to create project:", result.error); } } catch (error) { const errorMessage = error instanceof Error ? error.message : "An unexpected error occurred"; - alert(`Failed to add project: ${errorMessage}`); + setError(`Failed to add project: ${errorMessage}`); console.error("Failed to add project:", error); } }, [projects]); const removeProject = useCallback( async (path: string) => { + setError(null); try { const result = await window.api.projects.remove(path); if (result.success) { @@ -71,7 +69,7 @@ export function useProjectManagement() { } else { console.error("Failed to remove project:", result.error); // Show error to user - they might need to remove workspaces first - alert(result.error); + setError(result.error); } } catch (error) { console.error("Failed to remove project:", error); @@ -80,11 +78,17 @@ export function useProjectManagement() { [projects] ); + const clearError = useCallback(() => { + setError(null); + }, []); + return { projects, setProjects, addProject, removeProject, loadProjects, + error, + clearError, }; } From cc92fb6d47183a1a2859835a155a11ff788905ba Mon Sep 17 00:00:00 2001 From: Ammar Date: Thu, 30 Oct 2025 16:13:16 +0000 Subject: [PATCH 06/11] =?UTF-8?q?=F0=9F=A4=96=20refactor:=20use=20web=20di?= =?UTF-8?q?alog=20exclusively=20for=20project=20creation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove native Electron file dialog in favor of web dialog for consistency: - Update useProjectManagement to dispatch directory-select-request event - Remove dialog IPC handler (DIALOG_SELECT_DIR) from ipcMain - Remove dialog API from IPCApi interface, preload, and browser API - Update App.stories.tsx mock to remove dialog API - Remove DIALOG_SELECT_DIR constant from ipc-constants Both native and web versions now use DirectorySelectModal for path input, providing consistent UX across deployment modes. _Generated with `cmux`_ --- src/App.stories.tsx | 3 --- src/browser/api.ts | 19 ------------------- src/constants/ipc-constants.ts | 3 --- src/hooks/useProjectManagement.ts | 14 +++++++++++--- src/preload.ts | 3 --- src/services/ipcMain.ts | 21 --------------------- src/types/ipc.ts | 3 --- 7 files changed, 11 insertions(+), 55 deletions(-) diff --git a/src/App.stories.tsx b/src/App.stories.tsx index dc9d3f939..799620dde 100644 --- a/src/App.stories.tsx +++ b/src/App.stories.tsx @@ -19,9 +19,6 @@ function setupMockAPI(options: { const mockWorkspaces = options.workspaces ?? []; const mockApi: IPCApi = { - dialog: { - selectDirectory: () => Promise.resolve(null), - }, providers: { setProviderConfig: () => Promise.resolve({ success: true, data: undefined }), list: () => Promise.resolve([]), diff --git a/src/browser/api.ts b/src/browser/api.ts index 75e1c08e8..6a77917ab 100644 --- a/src/browser/api.ts +++ b/src/browser/api.ts @@ -184,27 +184,8 @@ class WebSocketManager { const wsManager = new WebSocketManager(); -// Directory selection via custom event (for browser mode) -interface DirectorySelectEvent extends CustomEvent { - detail: { - resolve: (path: string | null) => void; - }; -} - -function requestDirectorySelection(): Promise { - return new Promise((resolve) => { - const event = new CustomEvent("directory-select-request", { - detail: { resolve }, - }) as DirectorySelectEvent; - window.dispatchEvent(event); - }); -} - // Create the Web API implementation const webApi: IPCApi = { - dialog: { - selectDirectory: requestDirectorySelection, - }, providers: { setProviderConfig: (provider, keyPath, value) => invokeIPC(IPC_CHANNELS.PROVIDERS_SET_CONFIG, provider, keyPath, value), diff --git a/src/constants/ipc-constants.ts b/src/constants/ipc-constants.ts index ce3a3ffb0..b5d35489f 100644 --- a/src/constants/ipc-constants.ts +++ b/src/constants/ipc-constants.ts @@ -4,9 +4,6 @@ */ export const IPC_CHANNELS = { - // Dialog channels - DIALOG_SELECT_DIR: "dialog:selectDirectory", - // Provider channels PROVIDERS_SET_CONFIG: "providers:setConfig", PROVIDERS_LIST: "providers:list", diff --git a/src/hooks/useProjectManagement.ts b/src/hooks/useProjectManagement.ts index b54de6b68..d42de5358 100644 --- a/src/hooks/useProjectManagement.ts +++ b/src/hooks/useProjectManagement.ts @@ -25,10 +25,18 @@ export function useProjectManagement() { const addProject = useCallback(async () => { setError(null); - try { - const selectedPath = await window.api.dialog.selectDirectory(); - if (!selectedPath) return; + + // Request directory path via web dialog event + const selectedPath = await new Promise((resolve) => { + const event = new CustomEvent("directory-select-request", { + detail: { resolve }, + }); + window.dispatchEvent(event); + }); + + if (!selectedPath) return; + try { const result = await window.api.projects.create(selectedPath); if (result.success) { // Use the normalized path returned from backend diff --git a/src/preload.ts b/src/preload.ts index dfb2ad6b7..c767ced5f 100644 --- a/src/preload.ts +++ b/src/preload.ts @@ -26,9 +26,6 @@ import { IPC_CHANNELS, getChatChannel } from "./constants/ipc-constants"; // Build the API implementation using the shared interface const api: IPCApi = { - dialog: { - selectDirectory: () => ipcRenderer.invoke(IPC_CHANNELS.DIALOG_SELECT_DIR), - }, providers: { setProviderConfig: (provider, keyPath, value) => ipcRenderer.invoke(IPC_CHANNELS.PROVIDERS_SET_CONFIG, provider, keyPath, value), diff --git a/src/services/ipcMain.ts b/src/services/ipcMain.ts index 47879631f..ed2fa4845 100644 --- a/src/services/ipcMain.ts +++ b/src/services/ipcMain.ts @@ -217,7 +217,6 @@ export class IpcMain { return; } - this.registerDialogHandlers(ipcMain); this.registerWindowHandlers(ipcMain); this.registerWorkspaceHandlers(ipcMain); this.registerProviderHandlers(ipcMain); @@ -226,26 +225,6 @@ export class IpcMain { this.registered = true; } - private registerDialogHandlers(ipcMain: ElectronIpcMain): void { - ipcMain.handle(IPC_CHANNELS.DIALOG_SELECT_DIR, async () => { - if (!this.mainWindow) return null; - - // Dynamic import to avoid issues with electron mocks in tests - // eslint-disable-next-line no-restricted-syntax - const { dialog } = await import("electron"); - - const result = await dialog.showOpenDialog(this.mainWindow, { - properties: ["openDirectory"], - }); - - if (result.canceled) { - return null; - } - - return result.filePaths[0]; - }); - } - private registerWindowHandlers(ipcMain: ElectronIpcMain): void { ipcMain.handle(IPC_CHANNELS.WINDOW_SET_TITLE, (_event, title: string) => { if (!this.mainWindow) return; diff --git a/src/types/ipc.ts b/src/types/ipc.ts index cf7fd329f..d0c8c485c 100644 --- a/src/types/ipc.ts +++ b/src/types/ipc.ts @@ -200,9 +200,6 @@ export interface SendMessageOptions { // Minimize the number of methods - use optional parameters for operation variants // (e.g. remove(id, force?) not remove(id) + removeForce(id)). export interface IPCApi { - dialog: { - selectDirectory(): Promise; - }; providers: { setProviderConfig( provider: string, From f92adc6163d73a263e09777013d71723a9ea60d1 Mon Sep 17 00:00:00 2001 From: Ammar Date: Thu, 30 Oct 2025 16:23:04 +0000 Subject: [PATCH 07/11] =?UTF-8?q?=F0=9F=A4=96=20refactor:=20handle=20proje?= =?UTF-8?q?ct=20errors=20inline=20in=20DirectorySelectModal?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move error handling into DirectorySelectModal to keep modal open on error: - Modal now calls window.api.projects.create() directly - Backend validation errors displayed inline with path preserved - Duplicate project check moved to modal (using normalized path) - Remove ProjectErrorModal component (no longer needed) - Remove error state from useProjectManagement hook - Remove projectError/clearProjectError from AppContext - Add loading state with "Adding..." button text Modal stays open until project successfully created or user cancels. User can correct path without re-entering everything. _Generated with `cmux`_ --- src/App.tsx | 4 - src/components/AppLoader.tsx | 2 - src/components/DirectorySelectModal.tsx | 81 +++++++++++++++----- src/components/ProjectErrorModal.stories.tsx | 64 ---------------- src/components/ProjectErrorModal.tsx | 40 ---------- src/contexts/AppContext.tsx | 2 - src/hooks/useProjectManagement.ts | 54 ++++--------- 7 files changed, 76 insertions(+), 171 deletions(-) delete mode 100644 src/components/ProjectErrorModal.stories.tsx delete mode 100644 src/components/ProjectErrorModal.tsx diff --git a/src/App.tsx b/src/App.tsx index 5b518ecad..dbed6f47f 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -6,7 +6,6 @@ import type { FrontendWorkspaceMetadata } from "./types/workspace"; import { LeftSidebar } from "./components/LeftSidebar"; import NewWorkspaceModal from "./components/NewWorkspaceModal"; import { DirectorySelectModal } from "./components/DirectorySelectModal"; -import { ProjectErrorModal } from "./components/ProjectErrorModal"; import { AIView } from "./components/AIView"; import { ErrorBoundary } from "./components/ErrorBoundary"; import { usePersistedState, updatePersistedState } from "./hooks/usePersistedState"; @@ -39,8 +38,6 @@ function AppInner() { projects, addProject, removeProject, - projectError, - clearProjectError, workspaceMetadata, setWorkspaceMetadata, createWorkspace, @@ -698,7 +695,6 @@ function AppInner() { /> )} - ); diff --git a/src/components/AppLoader.tsx b/src/components/AppLoader.tsx index b41d124f8..739eee2c7 100644 --- a/src/components/AppLoader.tsx +++ b/src/components/AppLoader.tsx @@ -108,8 +108,6 @@ export function AppLoader() { setProjects={projectManagement.setProjects} addProject={projectManagement.addProject} removeProject={projectManagement.removeProject} - projectError={projectManagement.error} - clearProjectError={projectManagement.clearError} workspaceMetadata={workspaceManagement.workspaceMetadata} setWorkspaceMetadata={workspaceManagement.setWorkspaceMetadata} createWorkspace={workspaceManagement.createWorkspace} diff --git a/src/components/DirectorySelectModal.tsx b/src/components/DirectorySelectModal.tsx index d3b3a4dde..a64eb56e2 100644 --- a/src/components/DirectorySelectModal.tsx +++ b/src/components/DirectorySelectModal.tsx @@ -2,28 +2,31 @@ import React, { useState, useCallback, useEffect, useRef } from "react"; import { Modal, ModalActions, CancelButton, PrimaryButton } from "./Modal"; /** - * Self-contained directory selection modal for browser mode. + * Directory selection modal that handles the full project creation flow. * - * Listens for 'directory-select-request' custom events and displays - * a modal for the user to enter a directory path. The promise from - * the event is resolved with the selected path or null if cancelled. + * Listens for 'directory-select-request' custom events, displays a modal + * for path input, calls the backend to create the project, and shows + * validation errors inline. Modal stays open until project is successfully + * created or user cancels. */ export const DirectorySelectModal: React.FC = () => { const [isOpen, setIsOpen] = useState(false); const [path, setPath] = useState(""); const [error, setError] = useState(""); - const resolveRef = useRef<((path: string | null) => void) | null>(null); + const [isCreating, setIsCreating] = useState(false); + const resolveRef = useRef<((result: { success: boolean; data?: unknown }) => void) | null>(null); // Listen for directory selection requests useEffect(() => { const handleDirectorySelectRequest = (e: Event) => { const customEvent = e as CustomEvent<{ - resolve: (path: string | null) => void; + resolve: (result: { success: boolean; data?: unknown }) => void; }>; resolveRef.current = customEvent.detail.resolve; setPath(""); setError(""); + setIsCreating(false); setIsOpen(true); }; @@ -35,31 +38,64 @@ export const DirectorySelectModal: React.FC = () => { const handleCancel = useCallback(() => { if (resolveRef.current) { - resolveRef.current(null); + resolveRef.current({ success: false }); resolveRef.current = null; } setIsOpen(false); }, []); - const handleSelect = useCallback(() => { + const handleSelect = useCallback(async () => { const trimmedPath = path.trim(); if (!trimmedPath) { setError("Please enter a directory path"); return; } - if (resolveRef.current) { - resolveRef.current(trimmedPath); - resolveRef.current = null; + setError(""); + setIsCreating(true); + + try { + // First check if project already exists + const existingProjects = await window.api.projects.list(); + const existingPaths = new Map(existingProjects); + + // Try to create the project + const result = await window.api.projects.create(trimmedPath); + + if (result.success) { + // Check if duplicate (backend may normalize the path) + const { normalizedPath } = result.data as { normalizedPath: string }; + if (existingPaths.has(normalizedPath)) { + setError("This project has already been added."); + return; + } + + // Success - close modal and resolve + if (resolveRef.current) { + resolveRef.current({ success: true, data: result.data }); + resolveRef.current = null; + } + setIsOpen(false); + } else { + // Backend validation error - show inline, keep modal open + const errorMessage = + typeof result.error === "string" ? result.error : "Failed to add project"; + setError(errorMessage); + } + } catch (err) { + // Unexpected error + const errorMessage = err instanceof Error ? err.message : "An unexpected error occurred"; + setError(`Failed to add project: ${errorMessage}`); + } finally { + setIsCreating(false); } - setIsOpen(false); }, [path]); const handleKeyDown = useCallback( (e: React.KeyboardEvent) => { if (e.key === "Enter") { e.preventDefault(); - handleSelect(); + void handleSelect(); } }, [handleSelect] @@ -68,9 +104,10 @@ export const DirectorySelectModal: React.FC = () => { return ( { onKeyDown={handleKeyDown} placeholder="/home/user/projects/my-project" autoFocus - className="bg-modal-bg border-border-medium focus:border-accent placeholder:text-muted mb-5 w-full rounded border px-3 py-2 font-mono text-sm text-white focus:outline-none" + disabled={isCreating} + className="bg-modal-bg border-border-medium focus:border-accent placeholder:text-muted mb-5 w-full rounded border px-3 py-2 font-mono text-sm text-white focus:outline-none disabled:opacity-50" /> {error &&
{error}
} - Cancel - Select + + Cancel + + void handleSelect()} + disabled={isCreating} + > + {isCreating ? "Adding..." : "Add Project"} +
); diff --git a/src/components/ProjectErrorModal.stories.tsx b/src/components/ProjectErrorModal.stories.tsx deleted file mode 100644 index 2c7b9fc50..000000000 --- a/src/components/ProjectErrorModal.stories.tsx +++ /dev/null @@ -1,64 +0,0 @@ -import type { Meta, StoryObj } from "@storybook/react-vite"; -import { action } from "storybook/actions"; -import { ProjectErrorModal } from "./ProjectErrorModal"; - -const meta = { - title: "Components/ProjectErrorModal", - component: ProjectErrorModal, - parameters: { - layout: "fullscreen", - }, - tags: ["autodocs"], - argTypes: { - error: { - control: "text", - description: "Error message to display (null = modal closed)", - }, - onClose: { - control: false, - action: "onClose", - }, - }, - args: { - onClose: action("close-clicked"), - }, -} satisfies Meta; - -export default meta; -type Story = StoryObj; - -export const NotAGitRepository: Story = { - args: { - error: "Not a git repository: /home/user/my-project", - }, -}; - -export const PathDoesNotExist: Story = { - args: { - error: "Path does not exist: /home/user/nonexistent-project", - }, -}; - -export const PathIsNotDirectory: Story = { - args: { - error: "Path is not a directory: /home/user/file.txt", - }, -}; - -export const ProjectAlreadyExists: Story = { - args: { - error: "This project has already been added.", - }, -}; - -export const GenericError: Story = { - args: { - error: "Failed to add project: An unexpected error occurred", - }, -}; - -export const Closed: Story = { - args: { - error: null, - }, -}; diff --git a/src/components/ProjectErrorModal.tsx b/src/components/ProjectErrorModal.tsx deleted file mode 100644 index ebbef3a5b..000000000 --- a/src/components/ProjectErrorModal.tsx +++ /dev/null @@ -1,40 +0,0 @@ -import React, { useEffect } from "react"; -import { Modal, ModalActions, PrimaryButton } from "./Modal"; - -interface ProjectErrorModalProps { - error: string | null; - onClose: () => void; -} - -/** - * Modal for displaying project-related errors (add/remove failures, validation errors, etc.). - * Blocks the create flow until dismissed. - */ -export const ProjectErrorModal: React.FC = ({ error, onClose }) => { - // Auto-focus the close button for keyboard accessibility - useEffect(() => { - if (error) { - const handleEscape = (e: KeyboardEvent) => { - if (e.key === "Escape") { - onClose(); - } - }; - window.addEventListener("keydown", handleEscape); - return () => window.removeEventListener("keydown", handleEscape); - } - }, [error, onClose]); - - return ( - -
{error}
- - OK - -
- ); -}; diff --git a/src/contexts/AppContext.tsx b/src/contexts/AppContext.tsx index 92d9d3fef..5663dea1f 100644 --- a/src/contexts/AppContext.tsx +++ b/src/contexts/AppContext.tsx @@ -15,8 +15,6 @@ interface AppContextType { setProjects: Dispatch>>; addProject: () => Promise; removeProject: (path: string) => Promise; - projectError: string | null; - clearProjectError: () => void; // Workspaces workspaceMetadata: Map; diff --git a/src/hooks/useProjectManagement.ts b/src/hooks/useProjectManagement.ts index d42de5358..fd9ef1029 100644 --- a/src/hooks/useProjectManagement.ts +++ b/src/hooks/useProjectManagement.ts @@ -6,7 +6,6 @@ import type { ProjectConfig } from "@/config"; */ export function useProjectManagement() { const [projects, setProjects] = useState>(new Map()); - const [error, setError] = useState(null); useEffect(() => { void loadProjects(); @@ -24,50 +23,30 @@ export function useProjectManagement() { }; const addProject = useCallback(async () => { - setError(null); - - // Request directory path via web dialog event - const selectedPath = await new Promise((resolve) => { + // Request project creation via web dialog event + // Dialog handles the full flow: path input, validation, backend call, and error display + const result = await new Promise<{ success: boolean; data?: unknown }>((resolve) => { const event = new CustomEvent("directory-select-request", { detail: { resolve }, }); window.dispatchEvent(event); }); - if (!selectedPath) return; + if (!result.success || !result.data) return; - try { - const result = await window.api.projects.create(selectedPath); - if (result.success) { - // Use the normalized path returned from backend - const { normalizedPath, projectConfig } = result.data; - - // Check if already exists using normalized path - if (projects.has(normalizedPath)) { - setError("This project has already been added."); - return; - } + // Project was successfully created, add to local state + const { normalizedPath, projectConfig } = result.data as { + normalizedPath: string; + projectConfig: ProjectConfig; + }; - const newProjects = new Map(projects); - newProjects.set(normalizedPath, projectConfig); - setProjects(newProjects); - } else { - // Show error to user - const errorMessage = - typeof result.error === "string" ? result.error : "Failed to add project"; - setError(errorMessage); - console.error("Failed to create project:", result.error); - } - } catch (error) { - const errorMessage = error instanceof Error ? error.message : "An unexpected error occurred"; - setError(`Failed to add project: ${errorMessage}`); - console.error("Failed to add project:", error); - } + const newProjects = new Map(projects); + newProjects.set(normalizedPath, projectConfig); + setProjects(newProjects); }, [projects]); const removeProject = useCallback( async (path: string) => { - setError(null); try { const result = await window.api.projects.remove(path); if (result.success) { @@ -76,8 +55,7 @@ export function useProjectManagement() { setProjects(newProjects); } else { console.error("Failed to remove project:", result.error); - // Show error to user - they might need to remove workspaces first - setError(result.error); + // TODO: Show error to user in UI - they might need to remove workspaces first } } catch (error) { console.error("Failed to remove project:", error); @@ -86,17 +64,11 @@ export function useProjectManagement() { [projects] ); - const clearError = useCallback(() => { - setError(null); - }, []); - return { projects, setProjects, addProject, removeProject, loadProjects, - error, - clearError, }; } From df0763738c42348ab7cf62763bb6b7d04b28ee46 Mon Sep 17 00:00:00 2001 From: Ammar Date: Thu, 30 Oct 2025 16:25:39 +0000 Subject: [PATCH 08/11] =?UTF-8?q?=F0=9F=A4=96=20refactor:=20rename=20Direc?= =?UTF-8?q?torySelectModal=20to=20ProjectCreateModal?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename for better discoverability and clarity: - File: DirectorySelectModal.tsx → ProjectCreateModal.tsx - Component: DirectorySelectModal → ProjectCreateModal - Update imports in App.tsx - Update component documentation The modal now clearly indicates it handles project creation, not just directory selection. _Generated with `cmux`_ --- src/App.tsx | 4 ++-- .../{DirectorySelectModal.tsx => ProjectCreateModal.tsx} | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) rename src/components/{DirectorySelectModal.tsx => ProjectCreateModal.tsx} (96%) diff --git a/src/App.tsx b/src/App.tsx index dbed6f47f..4f4cc62a7 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -5,7 +5,7 @@ import type { WorkspaceSelection } from "./components/ProjectSidebar"; import type { FrontendWorkspaceMetadata } from "./types/workspace"; import { LeftSidebar } from "./components/LeftSidebar"; import NewWorkspaceModal from "./components/NewWorkspaceModal"; -import { DirectorySelectModal } from "./components/DirectorySelectModal"; +import { ProjectCreateModal } from "./components/ProjectCreateModal"; import { AIView } from "./components/AIView"; import { ErrorBoundary } from "./components/ErrorBoundary"; import { usePersistedState, updatePersistedState } from "./hooks/usePersistedState"; @@ -694,7 +694,7 @@ function AppInner() { onAdd={handleCreateWorkspace} /> )} - + ); diff --git a/src/components/DirectorySelectModal.tsx b/src/components/ProjectCreateModal.tsx similarity index 96% rename from src/components/DirectorySelectModal.tsx rename to src/components/ProjectCreateModal.tsx index a64eb56e2..7a75390b7 100644 --- a/src/components/DirectorySelectModal.tsx +++ b/src/components/ProjectCreateModal.tsx @@ -2,14 +2,14 @@ import React, { useState, useCallback, useEffect, useRef } from "react"; import { Modal, ModalActions, CancelButton, PrimaryButton } from "./Modal"; /** - * Directory selection modal that handles the full project creation flow. + * Project creation modal that handles the full flow from path input to backend validation. * * Listens for 'directory-select-request' custom events, displays a modal * for path input, calls the backend to create the project, and shows * validation errors inline. Modal stays open until project is successfully * created or user cancels. */ -export const DirectorySelectModal: React.FC = () => { +export const ProjectCreateModal: React.FC = () => { const [isOpen, setIsOpen] = useState(false); const [path, setPath] = useState(""); const [error, setError] = useState(""); From 371d5300859cd6959d64b7bd0983a6ca93a14205 Mon Sep 17 00:00:00 2001 From: Ammar Date: Thu, 30 Oct 2025 16:28:54 +0000 Subject: [PATCH 09/11] =?UTF-8?q?=F0=9F=A4=96=20refactor:=20remove=20event?= =?UTF-8?q?=20system,=20use=20props=20for=20ProjectCreateModal?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace custom event system with standard React props pattern: - ProjectCreateModal now accepts isOpen, onClose, onSuccess props - Remove event listener and promise-based communication - Update useProjectManagement.addProject to accept params directly - App.tsx manages projectCreateModalOpen state - Simpler flow: button click → setState(true) → modal opens Eliminates unnecessary indirection. Modal is just a normal React component controlled by parent state. _Generated with `cmux`_ --- src/App.tsx | 15 ++++-- src/components/ProjectCreateModal.tsx | 71 +++++++++++---------------- src/contexts/AppContext.tsx | 2 +- src/hooks/useProjectManagement.ts | 31 ++++-------- 4 files changed, 49 insertions(+), 70 deletions(-) diff --git a/src/App.tsx b/src/App.tsx index 4f4cc62a7..d7b1ea208 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -46,6 +46,7 @@ function AppInner() { selectedWorkspace, setSelectedWorkspace, } = useApp(); + const [projectCreateModalOpen, setProjectCreateModalOpen] = useState(false); const [workspaceModalOpen, setWorkspaceModalOpen] = useState(false); const [workspaceModalProject, setWorkspaceModalProject] = useState(null); const [workspaceModalProjectName, setWorkspaceModalProjectName] = useState(""); @@ -218,8 +219,8 @@ function AppInner() { // Memoize callbacks to prevent LeftSidebar/ProjectSidebar re-renders const handleAddProjectCallback = useCallback(() => { - void addProject(); - }, [addProject]); + setProjectCreateModalOpen(true); + }, []); const handleAddWorkspaceCallback = useCallback( (projectPath: string) => { @@ -473,8 +474,8 @@ function AppInner() { ); const addProjectFromPalette = useCallback(() => { - void addProject(); - }, [addProject]); + setProjectCreateModalOpen(true); + }, []); const removeProjectFromPalette = useCallback( (path: string) => { @@ -694,7 +695,11 @@ function AppInner() { onAdd={handleCreateWorkspace} /> )} - + setProjectCreateModalOpen(false)} + onSuccess={addProject} + /> ); diff --git a/src/components/ProjectCreateModal.tsx b/src/components/ProjectCreateModal.tsx index 7a75390b7..d2271465c 100644 --- a/src/components/ProjectCreateModal.tsx +++ b/src/components/ProjectCreateModal.tsx @@ -1,48 +1,33 @@ -import React, { useState, useCallback, useEffect, useRef } from "react"; +import React, { useState, useCallback } from "react"; import { Modal, ModalActions, CancelButton, PrimaryButton } from "./Modal"; +import type { ProjectConfig } from "@/config"; + +interface ProjectCreateModalProps { + isOpen: boolean; + onClose: () => void; + onSuccess: (normalizedPath: string, projectConfig: ProjectConfig) => void; +} /** * Project creation modal that handles the full flow from path input to backend validation. * - * Listens for 'directory-select-request' custom events, displays a modal - * for path input, calls the backend to create the project, and shows - * validation errors inline. Modal stays open until project is successfully - * created or user cancels. + * Displays a modal for path input, calls the backend to create the project, and shows + * validation errors inline. Modal stays open until project is successfully created or user cancels. */ -export const ProjectCreateModal: React.FC = () => { - const [isOpen, setIsOpen] = useState(false); +export const ProjectCreateModal: React.FC = ({ + isOpen, + onClose, + onSuccess, +}) => { const [path, setPath] = useState(""); const [error, setError] = useState(""); const [isCreating, setIsCreating] = useState(false); - const resolveRef = useRef<((result: { success: boolean; data?: unknown }) => void) | null>(null); - - // Listen for directory selection requests - useEffect(() => { - const handleDirectorySelectRequest = (e: Event) => { - const customEvent = e as CustomEvent<{ - resolve: (result: { success: boolean; data?: unknown }) => void; - }>; - - resolveRef.current = customEvent.detail.resolve; - setPath(""); - setError(""); - setIsCreating(false); - setIsOpen(true); - }; - - window.addEventListener("directory-select-request", handleDirectorySelectRequest); - return () => { - window.removeEventListener("directory-select-request", handleDirectorySelectRequest); - }; - }, []); const handleCancel = useCallback(() => { - if (resolveRef.current) { - resolveRef.current({ success: false }); - resolveRef.current = null; - } - setIsOpen(false); - }, []); + setPath(""); + setError(""); + onClose(); + }, [onClose]); const handleSelect = useCallback(async () => { const trimmedPath = path.trim(); @@ -64,18 +49,20 @@ export const ProjectCreateModal: React.FC = () => { if (result.success) { // Check if duplicate (backend may normalize the path) - const { normalizedPath } = result.data as { normalizedPath: string }; + const { normalizedPath, projectConfig } = result.data as { + normalizedPath: string; + projectConfig: ProjectConfig; + }; if (existingPaths.has(normalizedPath)) { setError("This project has already been added."); return; } - // Success - close modal and resolve - if (resolveRef.current) { - resolveRef.current({ success: true, data: result.data }); - resolveRef.current = null; - } - setIsOpen(false); + // Success - notify parent and close + onSuccess(normalizedPath, projectConfig); + setPath(""); + setError(""); + onClose(); } else { // Backend validation error - show inline, keep modal open const errorMessage = @@ -89,7 +76,7 @@ export const ProjectCreateModal: React.FC = () => { } finally { setIsCreating(false); } - }, [path]); + }, [path, onSuccess, onClose]); const handleKeyDown = useCallback( (e: React.KeyboardEvent) => { diff --git a/src/contexts/AppContext.tsx b/src/contexts/AppContext.tsx index 5663dea1f..f14600f73 100644 --- a/src/contexts/AppContext.tsx +++ b/src/contexts/AppContext.tsx @@ -13,7 +13,7 @@ interface AppContextType { // Projects projects: Map; setProjects: Dispatch>>; - addProject: () => Promise; + addProject: (normalizedPath: string, projectConfig: ProjectConfig) => void; removeProject: (path: string) => Promise; // Workspaces diff --git a/src/hooks/useProjectManagement.ts b/src/hooks/useProjectManagement.ts index fd9ef1029..2ad842b0a 100644 --- a/src/hooks/useProjectManagement.ts +++ b/src/hooks/useProjectManagement.ts @@ -22,28 +22,15 @@ export function useProjectManagement() { } }; - const addProject = useCallback(async () => { - // Request project creation via web dialog event - // Dialog handles the full flow: path input, validation, backend call, and error display - const result = await new Promise<{ success: boolean; data?: unknown }>((resolve) => { - const event = new CustomEvent("directory-select-request", { - detail: { resolve }, - }); - window.dispatchEvent(event); - }); - - if (!result.success || !result.data) return; - - // Project was successfully created, add to local state - const { normalizedPath, projectConfig } = result.data as { - normalizedPath: string; - projectConfig: ProjectConfig; - }; - - const newProjects = new Map(projects); - newProjects.set(normalizedPath, projectConfig); - setProjects(newProjects); - }, [projects]); + const addProject = useCallback( + (normalizedPath: string, projectConfig: ProjectConfig) => { + // Add successfully created project to local state + const newProjects = new Map(projects); + newProjects.set(normalizedPath, projectConfig); + setProjects(newProjects); + }, + [projects] + ); const removeProject = useCallback( async (path: string) => { From 9f1d217e4070bd10865bdaf9b610107466b5fc51 Mon Sep 17 00:00:00 2001 From: Ammar Date: Thu, 30 Oct 2025 16:32:53 +0000 Subject: [PATCH 10/11] =?UTF-8?q?=F0=9F=A4=96=20refactor:=20make=20validat?= =?UTF-8?q?eProjectPath=20async?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert validateProjectPath from sync to async to avoid blocking IPC handlers: - Use fs/promises instead of sync fs methods - Update IPC handlers to await validation - Update all tests to handle async function - Better error handling with try/catch instead of existsSync Removes eslint-disable comments for sync fs methods. _Generated with `cmux`_ --- src/services/ipcMain.ts | 6 ++-- src/utils/pathUtils.test.ts | 32 ++++++++++---------- src/utils/pathUtils.ts | 60 ++++++++++++++++++++----------------- 3 files changed, 52 insertions(+), 46 deletions(-) diff --git a/src/services/ipcMain.ts b/src/services/ipcMain.ts index ed2fa4845..440bb7009 100644 --- a/src/services/ipcMain.ts +++ b/src/services/ipcMain.ts @@ -1159,10 +1159,10 @@ export class IpcMain { } private registerProjectHandlers(ipcMain: ElectronIpcMain): void { - ipcMain.handle(IPC_CHANNELS.PROJECT_CREATE, (_event, projectPath: string) => { + ipcMain.handle(IPC_CHANNELS.PROJECT_CREATE, async (_event, projectPath: string) => { try { // Validate and expand path (handles tilde, checks existence and directory status) - const validation = validateProjectPath(projectPath); + const validation = await validateProjectPath(projectPath); if (!validation.valid) { return Err(validation.error ?? "Invalid project path"); } @@ -1247,7 +1247,7 @@ export class IpcMain { try { // Validate and expand path (handles tilde) - const validation = validateProjectPath(projectPath); + const validation = await validateProjectPath(projectPath); if (!validation.valid) { throw new Error(validation.error ?? "Invalid project path"); } diff --git a/src/utils/pathUtils.test.ts b/src/utils/pathUtils.test.ts index b9991ab29..2f8d86e7b 100644 --- a/src/utils/pathUtils.test.ts +++ b/src/utils/pathUtils.test.ts @@ -53,17 +53,17 @@ describe("pathUtils", () => { fs.rmSync(tempDir, { recursive: true, force: true }); }); - it("should return success for existing git directory", () => { + it("should return success for existing git directory", async () => { // Create .git directory // eslint-disable-next-line local/no-sync-fs-methods -- Test setup only fs.mkdirSync(path.join(tempDir, ".git")); - const result = validateProjectPath(tempDir); + const result = await validateProjectPath(tempDir); expect(result.valid).toBe(true); expect(result.expandedPath).toBe(tempDir); expect(result.error).toBeUndefined(); }); - it("should expand tilde and validate", () => { + it("should expand tilde and validate", async () => { // Create a test directory in home with .git const testDir = path.join(os.homedir(), `cmux-test-git-${Date.now()}`); // eslint-disable-next-line local/no-sync-fs-methods -- Test setup only @@ -71,7 +71,7 @@ describe("pathUtils", () => { // eslint-disable-next-line local/no-sync-fs-methods -- Test setup only fs.mkdirSync(path.join(testDir, ".git")); - const result = validateProjectPath(`~/${path.basename(testDir)}`); + const result = await validateProjectPath(`~/${path.basename(testDir)}`); expect(result.valid).toBe(true); expect(result.expandedPath).toBe(testDir); expect(result.error).toBeUndefined(); @@ -80,52 +80,52 @@ describe("pathUtils", () => { fs.rmSync(testDir, { recursive: true, force: true }); }); - it("should return error for non-existent path", () => { + it("should return error for non-existent path", async () => { const nonExistentPath = "/this/path/definitely/does/not/exist/cmux-test-12345"; - const result = validateProjectPath(nonExistentPath); + const result = await validateProjectPath(nonExistentPath); expect(result.valid).toBe(false); expect(result.error).toContain("does not exist"); }); - it("should return error for file path (not directory)", () => { + it("should return error for file path (not directory)", async () => { const filePath = path.join(tempDir, "test-file.txt"); // eslint-disable-next-line local/no-sync-fs-methods -- Test setup only fs.writeFileSync(filePath, "test content"); - const result = validateProjectPath(filePath); + const result = await validateProjectPath(filePath); expect(result.valid).toBe(false); expect(result.error).toContain("not a directory"); }); - it("should handle tilde path to non-existent directory", () => { + it("should handle tilde path to non-existent directory", async () => { const nonExistentTildePath = "~/this-directory-should-not-exist-cmux-test-12345"; - const result = validateProjectPath(nonExistentTildePath); + const result = await validateProjectPath(nonExistentTildePath); expect(result.valid).toBe(false); expect(result.error).toContain("does not exist"); }); - it("should return normalized absolute path", () => { + it("should return normalized absolute path", async () => { const pathWithDots = path.join(tempDir, "..", path.basename(tempDir)); // Add .git directory for validation // eslint-disable-next-line local/no-sync-fs-methods -- Test setup only fs.mkdirSync(path.join(tempDir, ".git")); - const result = validateProjectPath(pathWithDots); + const result = await validateProjectPath(pathWithDots); expect(result.valid).toBe(true); expect(result.expandedPath).toBe(tempDir); }); - it("should reject directory without .git", () => { - const result = validateProjectPath(tempDir); + it("should reject directory without .git", async () => { + const result = await validateProjectPath(tempDir); expect(result.valid).toBe(false); expect(result.error).toContain("Not a git repository"); }); - it("should accept directory with .git", () => { + it("should accept directory with .git", async () => { const gitDir = path.join(tempDir, ".git"); // eslint-disable-next-line local/no-sync-fs-methods -- Test setup only fs.mkdirSync(gitDir); - const result = validateProjectPath(tempDir); + const result = await validateProjectPath(tempDir); expect(result.valid).toBe(true); expect(result.expandedPath).toBe(tempDir); }); diff --git a/src/utils/pathUtils.ts b/src/utils/pathUtils.ts index fc505ebcd..e36a0224c 100644 --- a/src/utils/pathUtils.ts +++ b/src/utils/pathUtils.ts @@ -1,4 +1,4 @@ -import * as fs from "fs"; +import * as fs from "fs/promises"; import * as os from "os"; import * as path from "path"; @@ -46,16 +46,16 @@ export function expandTilde(inputPath: string): string { * @returns Validation result with expanded path or error * * @example - * validateProjectPath("~/my-project") + * await validateProjectPath("~/my-project") * // => { valid: true, expandedPath: "/home/user/my-project" } * - * validateProjectPath("~/nonexistent") + * await validateProjectPath("~/nonexistent") * // => { valid: false, error: "Path does not exist: /home/user/nonexistent" } * - * validateProjectPath("~/not-a-git-repo") + * await validateProjectPath("~/not-a-git-repo") * // => { valid: false, error: "Not a git repository: /home/user/not-a-git-repo" } */ -export function validateProjectPath(inputPath: string): PathValidationResult { +export async function validateProjectPath(inputPath: string): Promise { // Expand tilde if present const expandedPath = expandTilde(inputPath); @@ -63,32 +63,38 @@ export function validateProjectPath(inputPath: string): PathValidationResult { const normalizedPath = path.normalize(expandedPath); // Check if path exists - // eslint-disable-next-line local/no-sync-fs-methods -- Synchronous validation required for IPC handler - if (!fs.existsSync(normalizedPath)) { - return { - valid: false, - error: `Path does not exist: ${normalizedPath}`, - }; - } - - // Check if it's a directory - // eslint-disable-next-line local/no-sync-fs-methods -- Synchronous validation required for IPC handler - const stats = fs.statSync(normalizedPath); - if (!stats.isDirectory()) { - return { - valid: false, - error: `Path is not a directory: ${normalizedPath}`, - }; + try { + const stats = await fs.stat(normalizedPath); + + // Check if it's a directory + if (!stats.isDirectory()) { + return { + valid: false, + error: `Path is not a directory: ${normalizedPath}`, + }; + } + } catch (err) { + if ((err as NodeJS.ErrnoException).code === "ENOENT") { + return { + valid: false, + error: `Path does not exist: ${normalizedPath}`, + }; + } + throw err; } // Check if it's a git repository const gitPath = path.join(normalizedPath, ".git"); - // eslint-disable-next-line local/no-sync-fs-methods -- Synchronous validation required for IPC handler - if (!fs.existsSync(gitPath)) { - return { - valid: false, - error: `Not a git repository: ${normalizedPath}`, - }; + try { + await fs.stat(gitPath); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === "ENOENT") { + return { + valid: false, + error: `Not a git repository: ${normalizedPath}`, + }; + } + throw err; } return { From fc2dd04af3a29253c4debe979adeacc302837ee6 Mon Sep 17 00:00:00 2001 From: Ammar Date: Thu, 30 Oct 2025 16:35:55 +0000 Subject: [PATCH 11/11] =?UTF-8?q?=F0=9F=A4=96=20chore:=20format=20code?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _Generated with `cmux`_ --- src/components/ProjectCreateModal.tsx | 11 ++++------- src/utils/pathUtils.ts | 2 +- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/components/ProjectCreateModal.tsx b/src/components/ProjectCreateModal.tsx index d2271465c..6cb042678 100644 --- a/src/components/ProjectCreateModal.tsx +++ b/src/components/ProjectCreateModal.tsx @@ -43,10 +43,10 @@ export const ProjectCreateModal: React.FC = ({ // First check if project already exists const existingProjects = await window.api.projects.list(); const existingPaths = new Map(existingProjects); - + // Try to create the project const result = await window.api.projects.create(trimmedPath); - + if (result.success) { // Check if duplicate (backend may normalize the path) const { normalizedPath, projectConfig } = result.data as { @@ -57,7 +57,7 @@ export const ProjectCreateModal: React.FC = ({ setError("This project has already been added."); return; } - + // Success - notify parent and close onSuccess(normalizedPath, projectConfig); setPath(""); @@ -114,10 +114,7 @@ export const ProjectCreateModal: React.FC = ({ Cancel - void handleSelect()} - disabled={isCreating} - > + void handleSelect()} disabled={isCreating}> {isCreating ? "Adding..." : "Add Project"} diff --git a/src/utils/pathUtils.ts b/src/utils/pathUtils.ts index e36a0224c..4514280d6 100644 --- a/src/utils/pathUtils.ts +++ b/src/utils/pathUtils.ts @@ -65,7 +65,7 @@ export async function validateProjectPath(inputPath: string): Promise