fix: include pending workspaces in sidebar workspace list

ammar-agent · ammar-agent · commit 379f5844fdb3 · 2025-12-01T10:49:21.000-06:00
Extract buildSortedWorkspacesByProject utility to handle both persisted
and pending (status: creating) workspaces. Pending workspaces are now
displayed in the sidebar immediately when workspace creation starts,
rather than waiting for the config to be saved.

This fixes concurrent workspace creation where multiple workspaces
were being created simultaneously but only appeared after completion.
diff --git a/src/browser/App.tsx b/src/browser/App.tsx
@@ -3,13 +3,13 @@ import "./styles/globals.css";
 import { useWorkspaceContext } from "./contexts/WorkspaceContext";
 import { useProjectContext } from "./contexts/ProjectContext";
 import type { WorkspaceSelection } from "./components/ProjectSidebar";
-import type { FrontendWorkspaceMetadata } from "@/common/types/workspace";
 import { LeftSidebar } from "./components/LeftSidebar";
 import { ProjectCreateModal } from "./components/ProjectCreateModal";
 import { AIView } from "./components/AIView";
 import { ErrorBoundary } from "./components/ErrorBoundary";
 import { usePersistedState, updatePersistedState } from "./hooks/usePersistedState";
 import { matchesKeybind, KEYBINDS } from "./utils/ui/keybinds";
+import { buildSortedWorkspacesByProject } from "./utils/ui/workspaceFiltering";
 import { useResumeManager } from "./hooks/useResumeManager";
 import { useUnreadTracking } from "./hooks/useUnreadTracking";
 import { useWorkspaceStoreRaw, useWorkspaceRecency } from "./stores/WorkspaceStore";
@@ -198,46 +198,24 @@ function AppInner() {
   // NEW: Get workspace recency from store
   const workspaceRecency = useWorkspaceRecency();
 
-  // Sort workspaces by recency (most recent first)
-  // Returns Map<projectPath, FrontendWorkspaceMetadata[]> for direct component use
+  // Build sorted workspaces map including pending workspaces
   // Use stable reference to prevent sidebar re-renders when sort order hasn't changed
   const sortedWorkspacesByProject = useStableReference(
-    () => {
-      const result = new Map<string, FrontendWorkspaceMetadata[]>();
-      for (const [projectPath, config] of projects) {
-        // Transform Workspace[] to FrontendWorkspaceMetadata[] using workspace ID
-        const metadataList = config.workspaces
-          .map((ws) => (ws.id ? workspaceMetadata.get(ws.id) : undefined))
-          .filter((meta): meta is FrontendWorkspaceMetadata => meta !== undefined && meta !== null);
-
-        // Sort by recency
-        metadataList.sort((a, b) => {
-          const aTimestamp = workspaceRecency[a.id] ?? 0;
-          const bTimestamp = workspaceRecency[b.id] ?? 0;
-          return bTimestamp - aTimestamp;
+    () => buildSortedWorkspacesByProject(projects, workspaceMetadata, workspaceRecency),
+    (prev, next) =>
+      compareMaps(prev, next, (a, b) => {
+        if (a.length !== b.length) return false;
+        // Check ID, name, and status to detect changes
+        return a.every((meta, i) => {
+          const other = b[i];
+          return (
+            other &&
+            meta.id === other.id &&
+            meta.name === other.name &&
+            meta.status === other.status
+          );
         });
-
-        result.set(projectPath, metadataList);
-      }
-      return result;
-    },
-    (prev, next) => {
-      // Compare Maps: check if size, workspace order, and metadata content are the same
-      if (
-        !compareMaps(prev, next, (a, b) => {
-          if (a.length !== b.length) return false;
-          // Check both ID and name to detect renames
-          return a.every((metadata, i) => {
-            const bMeta = b[i];
-            if (!bMeta || !metadata) return false; // Null-safe
-            return metadata.id === bMeta.id && metadata.name === bMeta.name;
-          });
-        })
-      ) {
-        return false;
-      }
-      return true;
-    },
+      }),
     [projects, workspaceMetadata, workspaceRecency]
   );
 
@@ -597,12 +575,19 @@ function AppInner() {
                               new Map(prev).set(metadata.id, metadata)
                             );
 
-                            // Switch to new workspace
-                            setSelectedWorkspace({
-                              workspaceId: metadata.id,
-                              projectPath: metadata.projectPath,
-                              projectName: metadata.projectName,
-                              namedWorkspacePath: metadata.namedWorkspacePath,
+                            // Only switch to new workspace if user hasn't selected another one
+                            // during the creation process (selectedWorkspace was null when creation started)
+                            setSelectedWorkspace((current) => {
+                              if (current !== null) {
+                                // User has already selected another workspace - don't override
+                                return current;
+                              }
+                              return {
+                                workspaceId: metadata.id,
+                                projectPath: metadata.projectPath,
+                                projectName: metadata.projectName,
+                                namedWorkspacePath: metadata.namedWorkspacePath,
+                              };
                             });
 
                             // Track telemetry
diff --git a/src/browser/contexts/WorkspaceContext.tsx b/src/browser/contexts/WorkspaceContext.tsx
@@ -62,7 +62,7 @@ export interface WorkspaceContext {
 
   // Selection
   selectedWorkspace: WorkspaceSelection | null;
-  setSelectedWorkspace: (workspace: WorkspaceSelection | null) => void;
+  setSelectedWorkspace: React.Dispatch<React.SetStateAction<WorkspaceSelection | null>>;
 
   // Workspace creation flow
   pendingNewWorkspaceProject: string | null;
diff --git a/src/browser/utils/ui/workspaceFiltering.test.ts b/src/browser/utils/ui/workspaceFiltering.test.ts
@@ -1,6 +1,11 @@
 import { describe, it, expect } from "@jest/globals";
-import { partitionWorkspacesByAge, formatOldWorkspaceThreshold } from "./workspaceFiltering";
+import {
+  partitionWorkspacesByAge,
+  formatOldWorkspaceThreshold,
+  buildSortedWorkspacesByProject,
+} from "./workspaceFiltering";
 import type { FrontendWorkspaceMetadata } from "@/common/types/workspace";
+import type { ProjectConfig } from "@/common/types/project";
 import { DEFAULT_RUNTIME_CONFIG } from "@/common/constants/workspace";
 
 describe("partitionWorkspacesByAge", () => {
@@ -126,3 +131,145 @@ describe("formatOldWorkspaceThreshold", () => {
     expect(result).toBe("1 day");
   });
 });
+
+describe("buildSortedWorkspacesByProject", () => {
+  const createWorkspace = (
+    id: string,
+    projectPath: string,
+    status?: "creating"
+  ): FrontendWorkspaceMetadata => ({
+    id,
+    name: `workspace-${id}`,
+    projectName: projectPath.split("/").pop() ?? "unknown",
+    projectPath,
+    namedWorkspacePath: `${projectPath}/workspace-${id}`,
+    runtimeConfig: DEFAULT_RUNTIME_CONFIG,
+    status,
+  });
+
+  it("should include workspaces from persisted config", () => {
+    const projects = new Map<string, ProjectConfig>([
+      ["/project/a", { workspaces: [{ path: "/a/ws1", id: "ws1" }] }],
+    ]);
+    const metadata = new Map<string, FrontendWorkspaceMetadata>([
+      ["ws1", createWorkspace("ws1", "/project/a")],
+    ]);
+
+    const result = buildSortedWorkspacesByProject(projects, metadata, {});
+
+    expect(result.get("/project/a")).toHaveLength(1);
+    expect(result.get("/project/a")?.[0].id).toBe("ws1");
+  });
+
+  it("should include pending workspaces not yet in config", () => {
+    const projects = new Map<string, ProjectConfig>([
+      ["/project/a", { workspaces: [{ path: "/a/ws1", id: "ws1" }] }],
+    ]);
+    const metadata = new Map<string, FrontendWorkspaceMetadata>([
+      ["ws1", createWorkspace("ws1", "/project/a")],
+      ["pending1", createWorkspace("pending1", "/project/a", "creating")],
+    ]);
+
+    const result = buildSortedWorkspacesByProject(projects, metadata, {});
+
+    expect(result.get("/project/a")).toHaveLength(2);
+    expect(result.get("/project/a")?.map((w) => w.id)).toContain("ws1");
+    expect(result.get("/project/a")?.map((w) => w.id)).toContain("pending1");
+  });
+
+  it("should handle multiple concurrent pending workspaces", () => {
+    const projects = new Map<string, ProjectConfig>([["/project/a", { workspaces: [] }]]);
+    const metadata = new Map<string, FrontendWorkspaceMetadata>([
+      ["pending1", createWorkspace("pending1", "/project/a", "creating")],
+      ["pending2", createWorkspace("pending2", "/project/a", "creating")],
+      ["pending3", createWorkspace("pending3", "/project/a", "creating")],
+    ]);
+
+    const result = buildSortedWorkspacesByProject(projects, metadata, {});
+
+    expect(result.get("/project/a")).toHaveLength(3);
+  });
+
+  it("should add pending workspaces for projects not yet in config", () => {
+    const projects = new Map<string, ProjectConfig>();
+    const metadata = new Map<string, FrontendWorkspaceMetadata>([
+      ["pending1", createWorkspace("pending1", "/new/project", "creating")],
+    ]);
+
+    const result = buildSortedWorkspacesByProject(projects, metadata, {});
+
+    expect(result.get("/new/project")).toHaveLength(1);
+    expect(result.get("/new/project")?.[0].id).toBe("pending1");
+  });
+
+  it("should sort workspaces by recency (most recent first)", () => {
+    const now = Date.now();
+    const projects = new Map<string, ProjectConfig>([
+      [
+        "/project/a",
+        {
+          workspaces: [
+            { path: "/a/ws1", id: "ws1" },
+            { path: "/a/ws2", id: "ws2" },
+            { path: "/a/ws3", id: "ws3" },
+          ],
+        },
+      ],
+    ]);
+    const metadata = new Map<string, FrontendWorkspaceMetadata>([
+      ["ws1", createWorkspace("ws1", "/project/a")],
+      ["ws2", createWorkspace("ws2", "/project/a")],
+      ["ws3", createWorkspace("ws3", "/project/a")],
+    ]);
+    const recency = {
+      ws1: now - 3000, // oldest
+      ws2: now - 1000, // newest
+      ws3: now - 2000, // middle
+    };
+
+    const result = buildSortedWorkspacesByProject(projects, metadata, recency);
+
+    expect(result.get("/project/a")?.map((w) => w.id)).toEqual(["ws2", "ws3", "ws1"]);
+  });
+
+  it("should not duplicate workspaces that exist in both config and have creating status", () => {
+    // Edge case: workspace was saved to config but still has status: "creating"
+    // (this shouldn't happen in practice but tests defensive coding)
+    const projects = new Map<string, ProjectConfig>([
+      ["/project/a", { workspaces: [{ path: "/a/ws1", id: "ws1" }] }],
+    ]);
+    const metadata = new Map<string, FrontendWorkspaceMetadata>([
+      ["ws1", createWorkspace("ws1", "/project/a", "creating")],
+    ]);
+
+    const result = buildSortedWorkspacesByProject(projects, metadata, {});
+
+    expect(result.get("/project/a")).toHaveLength(1);
+    expect(result.get("/project/a")?.[0].id).toBe("ws1");
+  });
+
+  it("should skip workspaces with no id in config", () => {
+    const projects = new Map<string, ProjectConfig>([
+      ["/project/a", { workspaces: [{ path: "/a/legacy" }, { path: "/a/ws1", id: "ws1" }] }],
+    ]);
+    const metadata = new Map<string, FrontendWorkspaceMetadata>([
+      ["ws1", createWorkspace("ws1", "/project/a")],
+    ]);
+
+    const result = buildSortedWorkspacesByProject(projects, metadata, {});
+
+    expect(result.get("/project/a")).toHaveLength(1);
+    expect(result.get("/project/a")?.[0].id).toBe("ws1");
+  });
+
+  it("should skip config workspaces with no matching metadata", () => {
+    const projects = new Map<string, ProjectConfig>([
+      ["/project/a", { workspaces: [{ path: "/a/ws1", id: "ws1" }] }],
+    ]);
+    const metadata = new Map<string, FrontendWorkspaceMetadata>(); // empty
+
+    const result = buildSortedWorkspacesByProject(projects, metadata, {});
+
+    expect(result.get("/project/a")).toHaveLength(0);
+  });
+});
diff --git a/src/browser/utils/ui/workspaceFiltering.ts b/src/browser/utils/ui/workspaceFiltering.ts
@@ -1,10 +1,62 @@
 import type { FrontendWorkspaceMetadata } from "@/common/types/workspace";
+import type { ProjectConfig } from "@/common/types/project";
 
 /**
  * Time threshold for considering a workspace "old" (24 hours in milliseconds)
  */
 const OLD_WORKSPACE_THRESHOLD_MS = 24 * 60 * 60 * 1000;
 
+/**
+ * Build a map of project paths to sorted workspace metadata lists.
+ * Includes both persisted workspaces (from config) and pending workspaces
+ * (status: "creating") that haven't been saved yet.
+ *
+ * Workspaces are sorted by recency (most recent first).
+ */
+export function buildSortedWorkspacesByProject(
+  projects: Map<string, ProjectConfig>,
+  workspaceMetadata: Map<string, FrontendWorkspaceMetadata>,
+  workspaceRecency: Record<string, number>
+): Map<string, FrontendWorkspaceMetadata[]> {
+  const result = new Map<string, FrontendWorkspaceMetadata[]>();
+  const includedIds = new Set<string>();
+
+  // First pass: include workspaces from persisted config
+  for (const [projectPath, config] of projects) {
+    const metadataList: FrontendWorkspaceMetadata[] = [];
+    for (const ws of config.workspaces) {
+      if (!ws.id) continue;
+      const meta = workspaceMetadata.get(ws.id);
+      if (meta) {
+        metadataList.push(meta);
+        includedIds.add(ws.id);
+      }
+    }
+    result.set(projectPath, metadataList);
+  }
+
+  // Second pass: add pending workspaces (status: "creating") not yet in config
+  for (const [id, metadata] of workspaceMetadata) {
+    if (metadata.status === "creating" && !includedIds.has(id)) {
+      const projectWorkspaces = result.get(metadata.projectPath) ?? [];
+      projectWorkspaces.push(metadata);
+      result.set(metadata.projectPath, projectWorkspaces);
+    }
+  }
+
+  // Sort each project's workspaces by recency
+  for (const [projectPath, metadataList] of result) {
+    metadataList.sort((a, b) => {
+      const aTimestamp = workspaceRecency[a.id] ?? 0;
+      const bTimestamp = workspaceRecency[b.id] ?? 0;
+      return bTimestamp - aTimestamp;
+    });
+    result.set(projectPath, metadataList);
+  }
+
+  return result;
+}
+
 /**
  * Format the old workspace threshold for display.
  * Returns a human-readable string like "1 day", "2 hours", etc.
