From d493982925720bc7230aa163f84ff286eb36ed48 Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 24 Oct 2025 14:57:03 -0500 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=A4=96=20Fix=20misleading=20workspace?= =?UTF-8?q?=20ID=20comments=20and=20patterns?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Updated config.ts to clarify generateWorkspaceId is ONLY for legacy support - Fixed debug/git-status.ts to not suggest project-branch ID format - Updated App.stories.tsx mock IDs to use stable ID format (10 hex chars) - Added clarifying comments to E2E test fixtures - Documented that workspace IDs must come from backend, never constructed in frontend The generateWorkspaceId method is kept for: - Legacy workspace lookup (users may have old workspaces) - E2E test backward compatibility - Debug script legacy support --- src/App.stories.tsx | 16 +++++++++------- src/config.test.ts | 3 ++- src/config.ts | 7 ++++--- src/debug/agentSessionCli.ts | 2 ++ src/debug/git-status.ts | 5 ++++- src/debug/list-workspaces.ts | 4 +++- tests/e2e/utils/demoProject.ts | 3 ++- 7 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/App.stories.tsx b/src/App.stories.tsx index 0a44d052f..04d4fde97 100644 --- a/src/App.stories.tsx +++ b/src/App.stories.tsx @@ -31,7 +31,8 @@ function setupMockAPI(options: { Promise.resolve({ success: true, metadata: { - id: `${projectPath.split("/").pop() ?? "project"}-${branchName}`, + // Mock stable ID (production uses crypto.randomBytes(5).toString('hex')) + id: Math.random().toString(36).substring(2, 12), name: branchName, projectPath, projectName: projectPath.split("/").pop() ?? "project", @@ -135,15 +136,15 @@ export const SingleProject: Story = { "/home/user/projects/my-app", { workspaces: [ - { path: "/home/user/.cmux/src/my-app/main", id: "my-app-main", name: "main" }, + { path: "/home/user/.cmux/src/my-app/main", id: "a1b2c3d4e5", name: "main" }, { path: "/home/user/.cmux/src/my-app/feature-auth", - id: "my-app-feature-auth", + id: "f6g7h8i9j0", name: "feature/auth", }, { path: "/home/user/.cmux/src/my-app/bugfix", - id: "my-app-bugfix", + id: "k1l2m3n4o5", name: "bugfix/memory-leak", }, ], @@ -153,14 +154,14 @@ export const SingleProject: Story = { const workspaces: FrontendWorkspaceMetadata[] = [ { - id: "my-app-main", + id: "a1b2c3d4e5", name: "main", projectPath: "/home/user/projects/my-app", projectName: "my-app", namedWorkspacePath: "/home/user/.cmux/src/my-app/main", }, { - id: "my-app-feature-auth", + id: "f6g7h8i9j0", name: "feature/auth", projectPath: "/home/user/projects/my-app", projectName: "my-app", @@ -351,7 +352,8 @@ export const ActiveWorkspaceWithChat: Story = { Promise.resolve({ success: true, metadata: { - id: `${projectPath.split("/").pop() ?? "project"}-${branchName}`, + // Mock stable ID (production uses crypto.randomBytes(5).toString('hex')) + id: Math.random().toString(36).substring(2, 12), name: branchName, projectPath, projectName: projectPath.split("/").pop() ?? "project", diff --git a/src/config.test.ts b/src/config.test.ts index 23bac50ab..7e3c44c2f 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -79,7 +79,8 @@ describe("Config", () => { // Create workspace directory fs.mkdirSync(workspacePath, { recursive: true }); - // Create metadata file using legacy ID format (project-workspace) + // Test backward compatibility: Create metadata file using legacy ID format. + // This simulates workspaces created before stable IDs were introduced. const legacyId = config.generateWorkspaceId(projectPath, workspacePath); const sessionDir = config.getSessionDir(legacyId); fs.mkdirSync(sessionDir, { recursive: true }); diff --git a/src/config.ts b/src/config.ts index 790f861b1..2116dd434 100644 --- a/src/config.ts +++ b/src/config.ts @@ -112,10 +112,11 @@ export class Config { /** * DEPRECATED: Generate workspace ID from project and workspace paths. - * This method is used only for legacy workspace migration. - * New workspaces should use generateStableId() instead. + * This method is used only for legacy workspace migration to look up old workspaces. + * New workspaces use generateStableId() which returns a random stable ID. * - * Format: ${projectBasename}-${workspaceBasename} + * DO NOT use this method or its format to construct workspace IDs anywhere in the codebase. + * Workspace IDs are backend implementation details and must only come from backend operations. */ generateWorkspaceId(projectPath: string, workspacePath: string): string { const projectBasename = this.getProjectName(projectPath); diff --git a/src/debug/agentSessionCli.ts b/src/debug/agentSessionCli.ts index 6254d0027..bba8adcf7 100644 --- a/src/debug/agentSessionCli.ts +++ b/src/debug/agentSessionCli.ts @@ -170,6 +170,8 @@ async function main(): Promise { throw new Error("Provide --workspace-id or --project-path to derive workspace ID"); } const projectPath = path.resolve(projectPathRaw.trim()); + // Fallback to legacy ID format for looking up old workspaces. + // This is only used when --workspace-id is not explicitly provided. return config.generateWorkspaceId(projectPath, workspacePath); })(); diff --git a/src/debug/git-status.ts b/src/debug/git-status.ts index e5a7d4f62..8b90d1bc3 100644 --- a/src/debug/git-status.ts +++ b/src/debug/git-status.ts @@ -31,7 +31,10 @@ function findWorkspaces(): Array<{ id: string; path: string }> { const workspacePath = join(projectPath, branch); if (statSync(workspacePath).isDirectory()) { workspaces.push({ - id: `${project}-${branch}`, + // NOTE: Using directory name as display ID for debug purposes only. + // This is NOT how workspace IDs are determined in production code. + // Production workspace IDs come from metadata.json in the session dir. + id: branch, path: workspacePath, }); } diff --git a/src/debug/list-workspaces.ts b/src/debug/list-workspaces.ts index 2bfcc3b39..43017d650 100644 --- a/src/debug/list-workspaces.ts +++ b/src/debug/list-workspaces.ts @@ -15,8 +15,10 @@ export function listWorkspacesCommand() { console.log(` Workspaces: ${project.workspaces.length}`); for (const workspace of project.workspaces) { + // Note: This generates legacy-format IDs for display. Actual workspace IDs come from + // metadata.json (stable IDs) or config.json (migrated workspaces). const workspaceId = defaultConfig.generateWorkspaceId(projectPath, workspace.path); - console.log(` - ID: ${workspaceId} (generated)`); + console.log(` - Legacy ID: ${workspaceId} (for lookup only)`); console.log(` Path: ${workspace.path}`); console.log(` Exists: ${fs.existsSync(workspace.path)}`); } diff --git a/tests/e2e/utils/demoProject.ts b/tests/e2e/utils/demoProject.ts index 36cd183f8..a55611921 100644 --- a/tests/e2e/utils/demoProject.ts +++ b/tests/e2e/utils/demoProject.ts @@ -49,7 +49,8 @@ export function prepareDemoProject( fs.mkdirSync(workspacePath, { recursive: true }); fs.mkdirSync(sessionsDir, { recursive: true }); - // Workspace metadata mirrors Config.generateWorkspaceId + // E2E tests use legacy workspace ID format to test backward compatibility. + // Production code now uses generateStableId() for new workspaces. const config = new Config(rootDir); const workspaceId = config.generateWorkspaceId(projectPath, workspacePath); const metadata = { From 7621de54813571b6c51ecb9702784b6ccb93f111 Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 24 Oct 2025 15:07:57 -0500 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=A4=96=20Remove=20legacy=20ID=20suppo?= =?UTF-8?q?rt=20from=20debug=20commands?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Debug commands are internal dev tools and don't need backward compatibility. - debug/list-workspaces.ts: Display actual workspace ID from config instead of generating legacy IDs - debug/agentSessionCli.ts: Require --workspace-id instead of deriving from project path This eliminates any suggestion that workspace IDs can be constructed from project/branch names. --- src/debug/agentSessionCli.ts | 19 +++++-------------- src/debug/list-workspaces.ts | 12 ++++++++---- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/src/debug/agentSessionCli.ts b/src/debug/agentSessionCli.ts index bba8adcf7..5d0db5089 100644 --- a/src/debug/agentSessionCli.ts +++ b/src/debug/agentSessionCli.ts @@ -160,21 +160,12 @@ async function main(): Promise { const config = new Config(configRoot); const workspaceIdRaw = values["workspace-id"]; - const projectPathRaw = values["project-path"]; - - const workspaceId = - workspaceIdRaw && workspaceIdRaw.trim().length > 0 - ? workspaceIdRaw.trim() - : (() => { - if (typeof projectPathRaw !== "string" || projectPathRaw.trim().length === 0) { - throw new Error("Provide --workspace-id or --project-path to derive workspace ID"); - } - const projectPath = path.resolve(projectPathRaw.trim()); - // Fallback to legacy ID format for looking up old workspaces. - // This is only used when --workspace-id is not explicitly provided. - return config.generateWorkspaceId(projectPath, workspacePath); - })(); + if (typeof workspaceIdRaw !== "string" || workspaceIdRaw.trim().length === 0) { + throw new Error("--workspace-id is required"); + } + const workspaceId = workspaceIdRaw.trim(); + const projectPathRaw = values["project-path"]; const projectName = typeof projectPathRaw === "string" && projectPathRaw.trim().length > 0 ? path.basename(path.resolve(projectPathRaw.trim())) diff --git a/src/debug/list-workspaces.ts b/src/debug/list-workspaces.ts index 43017d650..9b7c4de8f 100644 --- a/src/debug/list-workspaces.ts +++ b/src/debug/list-workspaces.ts @@ -15,10 +15,14 @@ export function listWorkspacesCommand() { console.log(` Workspaces: ${project.workspaces.length}`); for (const workspace of project.workspaces) { - // Note: This generates legacy-format IDs for display. Actual workspace IDs come from - // metadata.json (stable IDs) or config.json (migrated workspaces). - const workspaceId = defaultConfig.generateWorkspaceId(projectPath, workspace.path); - console.log(` - Legacy ID: ${workspaceId} (for lookup only)`); + const dirName = path.basename(workspace.path); + console.log(` - Directory: ${dirName}`); + if (workspace.id) { + console.log(` ID: ${workspace.id}`); + } + if (workspace.name) { + console.log(` Name: ${workspace.name}`); + } console.log(` Path: ${workspace.path}`); console.log(` Exists: ${fs.existsSync(workspace.path)}`); } From 61bb0a957cab4ed50aa0970d3aa4e286c8b47ce7 Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 24 Oct 2025 15:28:35 -0500 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=A4=96=20Rename=20generateWorkspaceId?= =?UTF-8?q?=20to=20generateLegacyId?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Makes the legacy-only purpose explicit in the method name. - Renamed method in Config class - Updated all call sites in config.ts (4 locations) - Updated E2E test fixtures - Updated config.test.ts All tests pass. --- src/config.test.ts | 2 +- src/config.ts | 12 ++++++------ tests/e2e/utils/demoProject.ts | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/config.test.ts b/src/config.test.ts index 7e3c44c2f..ae6a62246 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -81,7 +81,7 @@ describe("Config", () => { // Test backward compatibility: Create metadata file using legacy ID format. // This simulates workspaces created before stable IDs were introduced. - const legacyId = config.generateWorkspaceId(projectPath, workspacePath); + const legacyId = config.generateLegacyId(projectPath, workspacePath); const sessionDir = config.getSessionDir(legacyId); fs.mkdirSync(sessionDir, { recursive: true }); const metadataPath = path.join(sessionDir, "metadata.json"); diff --git a/src/config.ts b/src/config.ts index 2116dd434..8562996ef 100644 --- a/src/config.ts +++ b/src/config.ts @@ -111,14 +111,14 @@ export class Config { } /** - * DEPRECATED: Generate workspace ID from project and workspace paths. + * DEPRECATED: Generate legacy workspace ID from project and workspace paths. * This method is used only for legacy workspace migration to look up old workspaces. * New workspaces use generateStableId() which returns a random stable ID. * * DO NOT use this method or its format to construct workspace IDs anywhere in the codebase. * Workspace IDs are backend implementation details and must only come from backend operations. */ - generateWorkspaceId(projectPath: string, workspacePath: string): string { + generateLegacyId(projectPath: string, workspacePath: string): string { const projectBasename = this.getProjectName(projectPath); const workspaceBasename = workspacePath.split("/").pop() ?? workspacePath.split("\\").pop() ?? "unknown"; @@ -198,7 +198,7 @@ export class Config { } // Try legacy ID format as last resort - const legacyId = this.generateWorkspaceId(projectPath, workspace.path); + const legacyId = this.generateLegacyId(projectPath, workspace.path); if (legacyId === workspaceId) { return { workspacePath: workspace.path, projectPath }; } @@ -273,7 +273,7 @@ export class Config { // LEGACY FORMAT: Fall back to reading metadata.json // Try legacy ID format first (project-workspace) - used by E2E tests and old workspaces - const legacyId = this.generateWorkspaceId(projectPath, workspace.path); + const legacyId = this.generateLegacyId(projectPath, workspace.path); const metadataPath = path.join(this.getSessionDir(legacyId), "metadata.json"); let metadataFound = false; @@ -303,7 +303,7 @@ export class Config { // No metadata found anywhere - create basic metadata if (!metadataFound) { - const legacyId = this.generateWorkspaceId(projectPath, workspace.path); + const legacyId = this.generateLegacyId(projectPath, workspace.path); const metadata: WorkspaceMetadata = { id: legacyId, name: workspaceBasename, @@ -321,7 +321,7 @@ export class Config { } catch (error) { console.error(`Failed to load/migrate workspace metadata:`, error); // Fallback to basic metadata if migration fails - const legacyId = this.generateWorkspaceId(projectPath, workspace.path); + const legacyId = this.generateLegacyId(projectPath, workspace.path); const metadata: WorkspaceMetadata = { id: legacyId, name: workspaceBasename, diff --git a/tests/e2e/utils/demoProject.ts b/tests/e2e/utils/demoProject.ts index a55611921..b1fc18198 100644 --- a/tests/e2e/utils/demoProject.ts +++ b/tests/e2e/utils/demoProject.ts @@ -52,7 +52,7 @@ export function prepareDemoProject( // E2E tests use legacy workspace ID format to test backward compatibility. // Production code now uses generateStableId() for new workspaces. const config = new Config(rootDir); - const workspaceId = config.generateWorkspaceId(projectPath, workspacePath); + const workspaceId = config.generateLegacyId(projectPath, workspacePath); const metadata = { id: workspaceId, name: workspaceBranch,