diff --git a/docs/AGENTS.md b/docs/AGENTS.md index 62184dbcd..55d8db0cd 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -178,6 +178,8 @@ This project uses **Make** as the primary build orchestrator. See `Makefile` for - When refactoring, use `git mv` to preserve file history instead of rewriting files from scratch +**⚠️ NEVER kill the running cmux process** - The main cmux instance is used for active development. Use `make test` or `make typecheck` to verify changes instead of starting the app in test worktrees. + ## Testing ### Test-Driven Development (TDD) diff --git a/src/App.stories.tsx b/src/App.stories.tsx index 1adbc8666..c68c21e53 100644 --- a/src/App.stories.tsx +++ b/src/App.stories.tsx @@ -1,6 +1,6 @@ import type { Meta, StoryObj } from "@storybook/react"; import { useRef } from "react"; -import App from "./App"; +import { AppLoader } from "./components/AppLoader"; import type { ProjectConfig } from "./config"; import type { FrontendWorkspaceMetadata } from "./types/workspace"; import type { IPCApi } from "./types/ipc"; @@ -94,7 +94,7 @@ function setupMockAPI(options: { const meta = { title: "App/Full Application", - component: App, + component: AppLoader, parameters: { layout: "fullscreen", backgrounds: { @@ -103,7 +103,7 @@ const meta = { }, }, tags: ["autodocs"], -} satisfies Meta; +} satisfies Meta; export default meta; type Story = StoryObj; @@ -122,7 +122,7 @@ const AppWithMocks: React.FC<{ initialized.current = true; } - return ; + return ; }; export const WelcomeScreen: Story = { @@ -618,7 +618,7 @@ export const ActiveWorkspaceWithChat: Story = { initialized.current = true; } - return ; + return ; }; return ; diff --git a/src/App.tsx b/src/App.tsx index 59bf74c6f..98918d7a1 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1,6 +1,6 @@ import { useState, useEffect, useCallback, useRef } from "react"; import "./styles/globals.css"; -import type { ProjectConfig } from "./config"; +import { useApp } from "./contexts/AppContext"; import type { WorkspaceSelection } from "./components/ProjectSidebar"; import type { FrontendWorkspaceMetadata } from "./types/workspace"; import { LeftSidebar } from "./components/LeftSidebar"; @@ -10,13 +10,10 @@ import { AIView } from "./components/AIView"; import { ErrorBoundary } from "./components/ErrorBoundary"; import { usePersistedState, updatePersistedState } from "./hooks/usePersistedState"; import { matchesKeybind, KEYBINDS } from "./utils/ui/keybinds"; -import { useProjectManagement } from "./hooks/useProjectManagement"; -import { useWorkspaceManagement } from "./hooks/useWorkspaceManagement"; import { useResumeManager } from "./hooks/useResumeManager"; import { useUnreadTracking } from "./hooks/useUnreadTracking"; import { useAutoCompactContinue } from "./hooks/useAutoCompactContinue"; import { useWorkspaceStoreRaw, useWorkspaceRecency } from "./stores/WorkspaceStore"; -import { useGitStatusStoreRaw } from "./stores/GitStatusStore"; import { useStableReference, compareMaps } from "./hooks/useStableReference"; import { CommandRegistryProvider, useCommandRegistry } from "./contexts/CommandRegistryContext"; @@ -34,10 +31,19 @@ import { useTelemetry } from "./hooks/useTelemetry"; const THINKING_LEVELS: ThinkingLevel[] = ["off", "low", "medium", "high"]; function AppInner() { - const [selectedWorkspace, setSelectedWorkspace] = usePersistedState( - "selectedWorkspace", - null - ); + // Get app-level state from context + const { + projects, + addProject, + removeProject, + workspaceMetadata, + setWorkspaceMetadata, + createWorkspace, + removeWorkspace, + renameWorkspace, + selectedWorkspace, + setSelectedWorkspace, + } = useApp(); const [workspaceModalOpen, setWorkspaceModalOpen] = useState(false); const [workspaceModalProject, setWorkspaceModalProject] = useState(null); const [workspaceModalProjectName, setWorkspaceModalProjectName] = useState(""); @@ -59,69 +65,33 @@ function AppInner() { // Telemetry tracking const telemetry = useTelemetry(); + // Get workspace store for command palette + const workspaceStore = useWorkspaceStoreRaw(); + // Wrapper for setSelectedWorkspace that tracks telemetry const handleWorkspaceSwitch = useCallback( (newWorkspace: WorkspaceSelection | null) => { - console.debug("[App] handleWorkspaceSwitch called", { - from: selectedWorkspace?.workspaceId, - to: newWorkspace?.workspaceId, - }); - // Track workspace switch when both old and new are non-null (actual switch, not init/clear) if ( selectedWorkspace && newWorkspace && selectedWorkspace.workspaceId !== newWorkspace.workspaceId ) { - console.debug("[App] Calling telemetry.workspaceSwitched"); telemetry.workspaceSwitched(selectedWorkspace.workspaceId, newWorkspace.workspaceId); } + setSelectedWorkspace(newWorkspace); }, [selectedWorkspace, setSelectedWorkspace, telemetry] ); - // Use custom hooks for project and workspace management - const { projects, setProjects, addProject, removeProject } = useProjectManagement(); - - // Workspace management needs to update projects state when workspace operations complete - const handleProjectsUpdate = useCallback( - (newProjects: Map) => { - setProjects(newProjects); - }, - [setProjects] - ); - - const { - workspaceMetadata, - setWorkspaceMetadata, - loading: metadataLoading, - createWorkspace, - removeWorkspace, - renameWorkspace, - } = useWorkspaceManagement({ - selectedWorkspace, - onProjectsUpdate: handleProjectsUpdate, - onSelectedWorkspaceUpdate: setSelectedWorkspace, - }); - - // NEW: Sync workspace metadata with the stores - const workspaceStore = useWorkspaceStoreRaw(); - const gitStatusStore = useGitStatusStoreRaw(); - - useEffect(() => { - // Only sync when metadata has actually loaded (not empty initial state) - if (workspaceMetadata.size > 0) { - workspaceStore.syncWorkspaces(workspaceMetadata); - } - }, [workspaceMetadata, workspaceStore]); - + // Validate selectedWorkspace when metadata changes + // Clear selection if workspace was deleted useEffect(() => { - // Only sync when metadata has actually loaded (not empty initial state) - if (workspaceMetadata.size > 0) { - gitStatusStore.syncWorkspaces(workspaceMetadata); + if (selectedWorkspace && !workspaceMetadata.has(selectedWorkspace.workspaceId)) { + setSelectedWorkspace(null); } - }, [workspaceMetadata, gitStatusStore]); + }, [selectedWorkspace, workspaceMetadata, setSelectedWorkspace]); // Track last-read timestamps for unread indicators const { lastReadTimestamps, onToggleUnread } = useUnreadTracking(selectedWorkspace); @@ -155,43 +125,8 @@ function AppInner() { } }, [selectedWorkspace, workspaceMetadata]); - // Restore workspace from URL on mount (if valid) - // This effect runs once on mount to restore from hash, which takes priority over localStorage - const [hasRestoredFromHash, setHasRestoredFromHash] = useState(false); - - useEffect(() => { - // Only run once - if (hasRestoredFromHash) return; - - // Wait for metadata to finish loading - if (metadataLoading) return; - - const hash = window.location.hash; - if (hash.startsWith("#workspace=")) { - const workspaceId = decodeURIComponent(hash.substring("#workspace=".length)); - - // Find workspace in metadata - const metadata = workspaceMetadata.get(workspaceId); - - if (metadata) { - // Restore from hash (overrides localStorage) - setSelectedWorkspace({ - workspaceId: metadata.id, - projectPath: metadata.projectPath, - projectName: metadata.projectName, - namedWorkspacePath: metadata.namedWorkspacePath, - }); - } - } - - setHasRestoredFromHash(true); - }, [metadataLoading, workspaceMetadata, hasRestoredFromHash, setSelectedWorkspace]); - // Validate selected workspace exists and has all required fields useEffect(() => { - // Don't validate until metadata is loaded - if (metadataLoading) return; - if (selectedWorkspace) { const metadata = workspaceMetadata.get(selectedWorkspace.workspaceId); @@ -215,7 +150,7 @@ function AppInner() { }); } } - }, [metadataLoading, selectedWorkspace, workspaceMetadata, setSelectedWorkspace]); + }, [selectedWorkspace, workspaceMetadata, setSelectedWorkspace]); const openWorkspaceInTerminal = useCallback( (workspaceId: string) => { @@ -635,6 +570,14 @@ function AppInner() { return; } + // DEFENSIVE: Ensure createdAt exists + if (!workspaceInfo.createdAt) { + console.warn( + `[Frontend] Workspace ${workspaceInfo.id} missing createdAt in fork switch - using default (2025-01-01)` + ); + workspaceInfo.createdAt = "2025-01-01T00:00:00.000Z"; + } + // Update metadata Map immediately (don't wait for async metadata event) // This ensures the title bar effect has the workspace name available setWorkspaceMetadata((prev) => { @@ -664,15 +607,10 @@ function AppInner() { <>
( + "selectedWorkspace", + null + ); + + // Load projects + const projectManagement = useProjectManagement(); + + // Load workspace metadata + // Pass empty callbacks for now - App will provide the actual handlers + const workspaceManagement = useWorkspaceManagement({ + selectedWorkspace, + onProjectsUpdate: projectManagement.setProjects, + onSelectedWorkspaceUpdate: setSelectedWorkspace, + }); + + // Get store instances + const workspaceStore = useWorkspaceStoreRaw(); + const gitStatusStore = useGitStatusStoreRaw(); + + // Track whether stores have been synced + const [storesSynced, setStoresSynced] = useState(false); + + // Sync stores when metadata finishes loading + useEffect(() => { + if (!workspaceManagement.loading) { + workspaceStore.syncWorkspaces(workspaceManagement.workspaceMetadata); + gitStatusStore.syncWorkspaces(workspaceManagement.workspaceMetadata); + setStoresSynced(true); + } else { + setStoresSynced(false); + } + }, [ + workspaceManagement.loading, + workspaceManagement.workspaceMetadata, + workspaceStore, + gitStatusStore, + ]); + + // Restore workspace from URL hash (runs once when stores are synced) + const [hasRestoredFromHash, setHasRestoredFromHash] = useState(false); + + useEffect(() => { + // Wait until stores are synced before attempting restoration + if (!storesSynced) return; + + // Only run once + if (hasRestoredFromHash) return; + + const hash = window.location.hash; + if (hash.startsWith("#workspace=")) { + const workspaceId = decodeURIComponent(hash.substring("#workspace=".length)); + + // Find workspace in metadata + const metadata = workspaceManagement.workspaceMetadata.get(workspaceId); + + if (metadata) { + // Restore from hash (overrides localStorage) + setSelectedWorkspace({ + workspaceId: metadata.id, + projectPath: metadata.projectPath, + projectName: metadata.projectName, + namedWorkspacePath: metadata.namedWorkspacePath, + }); + } + } + + setHasRestoredFromHash(true); + }, [ + storesSynced, + workspaceManagement.workspaceMetadata, + hasRestoredFromHash, + setSelectedWorkspace, + ]); + + // Show loading screen until stores are synced + if (workspaceManagement.loading || !storesSynced) { + return ; + } + + // Render App with all initialized data via context + return ( + + + + ); +} diff --git a/src/components/LeftSidebar.tsx b/src/components/LeftSidebar.tsx index 86799387c..2232f2860 100644 --- a/src/components/LeftSidebar.tsx +++ b/src/components/LeftSidebar.tsx @@ -1,28 +1,17 @@ import React from "react"; import { cn } from "@/lib/utils"; -import type { ProjectConfig } from "@/config"; import type { FrontendWorkspaceMetadata } from "@/types/workspace"; -import type { WorkspaceSelection } from "./ProjectSidebar"; import type { Secret } from "@/types/secrets"; import ProjectSidebar from "./ProjectSidebar"; import { TitleBar } from "./TitleBar"; +import { useApp } from "@/contexts/AppContext"; +import type { WorkspaceSelection } from "./ProjectSidebar"; interface LeftSidebarProps { - projects: Map; - workspaceMetadata: Map; - selectedWorkspace: WorkspaceSelection | null; onSelectWorkspace: (selection: WorkspaceSelection) => void; onAddProject: () => void; onAddWorkspace: (projectPath: string) => void; onRemoveProject: (path: string) => void; - onRemoveWorkspace: ( - workspaceId: string, - options?: { force?: boolean } - ) => Promise<{ success: boolean; error?: string }>; - onRenameWorkspace: ( - workspaceId: string, - newName: string - ) => Promise<{ success: boolean; error?: string }>; lastReadTimestamps: Record; onToggleUnread: (workspaceId: string) => void; collapsed: boolean; @@ -36,6 +25,10 @@ interface LeftSidebarProps { export function LeftSidebar(props: LeftSidebarProps) { const { collapsed, onToggleCollapsed, ...projectSidebarProps } = props; + // Get app-level state from context + const { projects, workspaceMetadata, selectedWorkspace, removeWorkspace, renameWorkspace } = + useApp(); + return ( <> {/* Hamburger menu button - only visible on mobile */} @@ -82,6 +75,11 @@ export function LeftSidebar(props: LeftSidebarProps) { {!collapsed && } diff --git a/src/components/LoadingScreen.tsx b/src/components/LoadingScreen.tsx new file mode 100644 index 000000000..6b249a373 --- /dev/null +++ b/src/components/LoadingScreen.tsx @@ -0,0 +1,10 @@ +export function LoadingScreen() { + return ( +
+
+
+

Loading workspaces...

+
+
+ ); +} diff --git a/src/config.ts b/src/config.ts index df825ae8c..2a8ab46f1 100644 --- a/src/config.ts +++ b/src/config.ts @@ -246,6 +246,10 @@ export class Config { * * This centralizes workspace metadata in config.json and eliminates the need * for scattered metadata.json files (kept for backward compat with older versions). + * + * GUARANTEE: Every workspace returned will have a createdAt timestamp. + * If missing from config or legacy metadata, a new timestamp is assigned and + * saved to config for subsequent loads. */ getAllWorkspaceMetadata(): FrontendWorkspaceMetadata[] { const config = this.loadConfigOrDefault(); @@ -268,8 +272,16 @@ export class Config { name: workspace.name, projectName, projectPath, - createdAt: workspace.createdAt, + // GUARANTEE: All workspaces must have createdAt (assign now if missing) + createdAt: workspace.createdAt ?? new Date().toISOString(), }; + + // Migrate missing createdAt to config for next load + if (!workspace.createdAt) { + workspace.createdAt = metadata.createdAt; + configModified = true; + } + workspaceMetadata.push(this.addPathsToMetadata(metadata, workspace.path, projectPath)); continue; // Skip metadata file lookup } @@ -294,6 +306,9 @@ export class Config { }; } + // GUARANTEE: All workspaces must have createdAt + metadata.createdAt ??= new Date().toISOString(); + // Migrate to config for next load workspace.id = metadata.id; workspace.name = metadata.name; @@ -312,11 +327,14 @@ export class Config { name: workspaceBasename, projectName, projectPath, + // GUARANTEE: All workspaces must have createdAt + createdAt: new Date().toISOString(), }; // Save to config for next load workspace.id = metadata.id; workspace.name = metadata.name; + workspace.createdAt = metadata.createdAt; configModified = true; workspaceMetadata.push(this.addPathsToMetadata(metadata, workspace.path, projectPath)); @@ -330,6 +348,8 @@ export class Config { name: workspaceBasename, projectName, projectPath, + // GUARANTEE: All workspaces must have createdAt (even in error cases) + createdAt: new Date().toISOString(), }; workspaceMetadata.push(this.addPathsToMetadata(metadata, workspace.path, projectPath)); } diff --git a/src/contexts/AppContext.tsx b/src/contexts/AppContext.tsx new file mode 100644 index 000000000..4a7bbbcb3 --- /dev/null +++ b/src/contexts/AppContext.tsx @@ -0,0 +1,61 @@ +import type { ReactNode, Dispatch, SetStateAction } from "react"; +import { createContext, useContext } from "react"; +import type { ProjectConfig } from "@/config"; +import type { FrontendWorkspaceMetadata } from "@/types/workspace"; +import type { WorkspaceSelection } from "@/components/ProjectSidebar"; + +/** + * App-level state and operations shared across the component tree. + * Eliminates prop drilling for common data like projects, workspaces, and selection. + */ +interface AppContextType { + // Projects + projects: Map; + setProjects: Dispatch>>; + addProject: () => Promise; + removeProject: (path: string) => Promise; + + // Workspaces + workspaceMetadata: Map; + setWorkspaceMetadata: Dispatch>>; + createWorkspace: ( + projectPath: string, + branchName: string, + trunkBranch: string + ) => Promise<{ + projectPath: string; + projectName: string; + namedWorkspacePath: string; + workspaceId: string; + }>; + removeWorkspace: ( + workspaceId: string, + options?: { force?: boolean } + ) => Promise<{ success: boolean; error?: string }>; + renameWorkspace: ( + workspaceId: string, + newName: string + ) => Promise<{ success: boolean; error?: string }>; + + // Selection + selectedWorkspace: WorkspaceSelection | null; + setSelectedWorkspace: (workspace: WorkspaceSelection | null) => void; +} + +const AppContext = createContext(undefined); + +interface AppProviderProps extends AppContextType { + children: ReactNode; +} + +export const AppProvider: React.FC = ({ children, ...value }) => { + return {children}; +}; + +export const useApp = (): AppContextType => { + const context = useContext(AppContext); + if (!context) { + throw new Error("useApp must be used within AppProvider"); + } + return context; +}; diff --git a/src/hooks/useWorkspaceManagement.ts b/src/hooks/useWorkspaceManagement.ts index 5b3e5bb51..b0bd62a16 100644 --- a/src/hooks/useWorkspaceManagement.ts +++ b/src/hooks/useWorkspaceManagement.ts @@ -10,6 +10,20 @@ interface UseWorkspaceManagementProps { onSelectedWorkspaceUpdate: (workspace: WorkspaceSelection | null) => void; } +/** + * Ensure workspace metadata has createdAt timestamp. + * DEFENSIVE: Backend guarantees createdAt, but default to 2025-01-01 if missing. + * This prevents crashes if backend contract is violated. + */ +function ensureCreatedAt(metadata: FrontendWorkspaceMetadata): void { + if (!metadata.createdAt) { + console.warn( + `[Frontend] Workspace ${metadata.id} missing createdAt - using default (2025-01-01)` + ); + metadata.createdAt = "2025-01-01T00:00:00.000Z"; + } +} + /** * Hook to manage workspace operations (create, remove, rename, list) */ @@ -28,6 +42,7 @@ export function useWorkspaceManagement({ const metadataList = await window.api.workspace.list(); const metadataMap = new Map(); for (const metadata of metadataList) { + ensureCreatedAt(metadata); // Use stable workspace ID as key (not path, which can change) metadataMap.set(metadata.id, metadata); } @@ -62,6 +77,7 @@ export function useWorkspaceManagement({ // Workspace deleted - remove from map updated.delete(event.workspaceId); } else { + ensureCreatedAt(event.metadata); updated.set(event.workspaceId, event.metadata); } @@ -169,6 +185,7 @@ export function useWorkspaceManagement({ // Get updated workspace metadata from backend const newMetadata = await window.api.workspace.getInfo(newWorkspaceId); if (newMetadata) { + ensureCreatedAt(newMetadata); onSelectedWorkspaceUpdate({ projectPath: selectedWorkspace.projectPath, projectName: newMetadata.projectName, diff --git a/src/main.tsx b/src/main.tsx index edcc218ac..e37858de6 100644 --- a/src/main.tsx +++ b/src/main.tsx @@ -1,6 +1,6 @@ import React from "react"; import ReactDOM from "react-dom/client"; -import App from "./App"; +import { AppLoader } from "./components/AppLoader"; import { initTelemetry, trackAppStarted } from "./telemetry"; // Shims the `window.api` object with the browser API. @@ -35,7 +35,7 @@ window.addEventListener("unhandledrejection", (event) => { ReactDOM.createRoot(document.getElementById("root")!).render( - + ); diff --git a/src/stores/WorkspaceStore.test.ts b/src/stores/WorkspaceStore.test.ts index a10d7064b..c29f73205 100644 --- a/src/stores/WorkspaceStore.test.ts +++ b/src/stores/WorkspaceStore.test.ts @@ -41,6 +41,24 @@ function getOnChatCallback(): (data: T) => void { return mock.mock.calls[0][1]; } +// Helper to create and add a workspace +function createAndAddWorkspace( + store: WorkspaceStore, + workspaceId: string, + options: Partial = {} +): FrontendWorkspaceMetadata { + const metadata: FrontendWorkspaceMetadata = { + id: workspaceId, + name: options.name ?? `test-branch-${workspaceId}`, + projectName: options.projectName ?? "test-project", + projectPath: options.projectPath ?? "/path/to/project", + namedWorkspacePath: options.namedWorkspacePath ?? "/path/to/worktree", + createdAt: options.createdAt ?? new Date().toISOString(), + }; + store.addWorkspace(metadata); + return metadata; +} + describe("WorkspaceStore", () => { let store: WorkspaceStore; let mockOnModelUsed: jest.Mock; @@ -56,6 +74,72 @@ describe("WorkspaceStore", () => { store.dispose(); }); + describe("recency calculation for new workspaces", () => { + it("should calculate recency from createdAt when workspace is added", () => { + const workspaceId = "test-workspace"; + const createdAt = new Date().toISOString(); + const metadata: FrontendWorkspaceMetadata = { + id: workspaceId, + name: "test-branch", + projectName: "test-project", + projectPath: "/path/to/project", + namedWorkspacePath: "/path/to/worktree", + createdAt, + }; + + // Add workspace with createdAt + store.addWorkspace(metadata); + + // Get state - should have recency based on createdAt + const state = store.getWorkspaceState(workspaceId); + + // Recency should be based on createdAt, not null or 0 + expect(state.recencyTimestamp).not.toBeNull(); + expect(state.recencyTimestamp).toBe(new Date(createdAt).getTime()); + + // Check that workspace appears in recency map with correct timestamp + const recency = store.getWorkspaceRecency(); + expect(recency[workspaceId]).toBe(new Date(createdAt).getTime()); + }); + + it("should maintain createdAt-based recency after CAUGHT_UP with no messages", async () => { + const workspaceId = "test-workspace-2"; + const createdAt = new Date().toISOString(); + const metadata: FrontendWorkspaceMetadata = { + id: workspaceId, + name: "test-branch-2", + projectName: "test-project", + projectPath: "/path/to/project", + namedWorkspacePath: "/path/to/worktree", + createdAt, + }; + + // Add workspace + store.addWorkspace(metadata); + + // Check initial recency + const initialState = store.getWorkspaceState(workspaceId); + expect(initialState.recencyTimestamp).toBe(new Date(createdAt).getTime()); + + // Get the IPC callback to simulate messages + const callback = getOnChatCallback(); + + // Simulate CAUGHT_UP message with no history (new workspace with no messages) + callback({ type: "caught-up" }); + + // Wait for async processing + await new Promise((resolve) => setTimeout(resolve, 10)); + + // Recency should still be based on createdAt + const stateAfterCaughtUp = store.getWorkspaceState(workspaceId); + expect(stateAfterCaughtUp.recencyTimestamp).toBe(new Date(createdAt).getTime()); + + // Verify recency map + const recency = store.getWorkspaceRecency(); + expect(recency[workspaceId]).toBe(new Date(createdAt).getTime()); + }); + }); + describe("subscription", () => { it("should call listener when workspace state changes", async () => { const listener = jest.fn(); @@ -68,6 +152,7 @@ describe("WorkspaceStore", () => { projectName: "test-project", projectPath: "/test/project", namedWorkspacePath: "/test/project/test-workspace", + createdAt: new Date().toISOString(), }; // Add workspace (should trigger IPC subscription) @@ -95,6 +180,7 @@ describe("WorkspaceStore", () => { projectName: "test-project", projectPath: "/test/project", namedWorkspacePath: "/test/project/test-workspace", + createdAt: new Date().toISOString(), }; store.addWorkspace(metadata); @@ -117,6 +203,7 @@ describe("WorkspaceStore", () => { projectName: "project-1", projectPath: "/project-1", namedWorkspacePath: "/path/1", + createdAt: new Date().toISOString(), }; const workspaceMap = new Map([[metadata1.id, metadata1]]); @@ -135,6 +222,7 @@ describe("WorkspaceStore", () => { projectName: "project-1", projectPath: "/project-1", namedWorkspacePath: "/path/1", + createdAt: new Date().toISOString(), }; // Add workspace @@ -151,7 +239,8 @@ describe("WorkspaceStore", () => { }); describe("getWorkspaceState", () => { - it("should return default state for new workspace", () => { + it("should return initial state for newly added workspace", () => { + createAndAddWorkspace(store, "new-workspace"); const state = store.getWorkspaceState("new-workspace"); expect(state).toMatchObject({ @@ -161,11 +250,13 @@ describe("WorkspaceStore", () => { loading: true, // loading because not caught up cmuxMessages: [], currentModel: null, - recencyTimestamp: null, }); + // Should have recency based on createdAt + expect(state.recencyTimestamp).not.toBeNull(); }); it("should return cached state when values unchanged", () => { + createAndAddWorkspace(store, "test-workspace"); const state1 = store.getWorkspaceState("test-workspace"); const state2 = store.getWorkspaceState("test-workspace"); @@ -197,6 +288,7 @@ describe("WorkspaceStore", () => { projectName: "test-project", projectPath: "/test/project", namedWorkspacePath: "/test/project/test-workspace", + createdAt: new Date().toISOString(), }; store.addWorkspace(metadata); @@ -241,6 +333,7 @@ describe("WorkspaceStore", () => { projectName: "test-project", projectPath: "/test/project", namedWorkspacePath: "/test/project/test-workspace", + createdAt: new Date().toISOString(), }; store.addWorkspace(metadata); @@ -268,6 +361,9 @@ describe("WorkspaceStore", () => { emitCount++; }); + // Add workspace first + createAndAddWorkspace(store, "test-workspace"); + // Simulate what happens during render - component calls getAggregator const aggregator1 = store.getAggregator("test-workspace"); expect(aggregator1).toBeDefined(); @@ -292,6 +388,7 @@ describe("WorkspaceStore", () => { projectName: "test-project", projectPath: "/test/project", namedWorkspacePath: "/test/project/test-workspace", + createdAt: new Date().toISOString(), }; store.addWorkspace(metadata); @@ -330,6 +427,7 @@ describe("WorkspaceStore", () => { projectName: "test-project", projectPath: "/test/project", namedWorkspacePath: "/test/project/test-workspace", + createdAt: new Date().toISOString(), }; store.addWorkspace(metadata); @@ -360,27 +458,22 @@ describe("WorkspaceStore", () => { expect(states1).not.toBe(states2); // Cache should be invalidated }); - it("invalidates getWorkspaceRecency() cache when workspace changes", async () => { + it("maintains recency based on createdAt for new workspaces", () => { + const createdAt = new Date("2024-01-01T00:00:00Z").toISOString(); const metadata: FrontendWorkspaceMetadata = { id: "test-workspace", name: "test-workspace", projectName: "test-project", projectPath: "/test/project", namedWorkspacePath: "/test/project/test-workspace", + createdAt, }; store.addWorkspace(metadata); - const recency1 = store.getWorkspaceRecency(); - - // Trigger change (caught-up message) - const onChatCallback = getOnChatCallback(); - onChatCallback({ type: "caught-up" }); - - // Wait for queueMicrotask to complete - await new Promise((resolve) => setTimeout(resolve, 0)); + const recency = store.getWorkspaceRecency(); - const recency2 = store.getWorkspaceRecency(); - expect(recency1).not.toBe(recency2); // Cache should be invalidated + // Recency should be based on createdAt + expect(recency["test-workspace"]).toBe(new Date(createdAt).getTime()); }); it("maintains cache when no changes occur", () => { @@ -390,6 +483,7 @@ describe("WorkspaceStore", () => { projectName: "test-project", projectPath: "/test/project", namedWorkspacePath: "/test/project/test-workspace", + createdAt: new Date().toISOString(), }; store.addWorkspace(metadata); @@ -411,50 +505,32 @@ describe("WorkspaceStore", () => { }); describe("race conditions", () => { - it("handles IPC message for removed workspace gracefully", async () => { + it("properly cleans up workspace on removal", () => { const metadata: FrontendWorkspaceMetadata = { id: "test-workspace", name: "test-workspace", projectName: "test-project", projectPath: "/test/project", namedWorkspacePath: "/test/project/test-workspace", + createdAt: new Date().toISOString(), }; store.addWorkspace(metadata); - const onChatCallback = getOnChatCallback(); + // Verify workspace exists + let allStates = store.getAllStates(); + expect(allStates.size).toBe(1); // Remove workspace (clears aggregator and unsubscribes IPC) store.removeWorkspace("test-workspace"); - // IPC message arrives after removal - should not throw - // Note: In practice, the IPC unsubscribe should prevent this, - // but if a message was already queued, it should handle gracefully - const onChatCallbackTyped = onChatCallback as (data: { - type: string; - messageId?: string; - model?: string; - }) => void; - expect(() => { - // Mark as caught-up first - onChatCallbackTyped({ - type: "caught-up", - }); - onChatCallbackTyped({ - type: "stream-start", - messageId: "msg1", - model: "claude-sonnet-4", - }); - }).not.toThrow(); + // Verify workspace is completely removed + allStates = store.getAllStates(); + expect(allStates.size).toBe(0); - // Wait for queueMicrotask to complete - await new Promise((resolve) => setTimeout(resolve, 0)); - - // The message handler will have created a new aggregator (lazy init) - // because getOrCreateAggregator always creates if not exists. - // This is actually fine - the workspace just has no IPC subscription. - const allStates = store.getAllStates(); - expect(allStates.size).toBe(1); // Aggregator exists but not subscribed - expect(allStates.get("test-workspace")?.canInterrupt).toBe(true); // Stream started + // Verify aggregator is gone + expect(() => store.getAggregator("test-workspace")).toThrow( + /Workspace test-workspace not found/ + ); }); it("handles concurrent workspace additions", () => { @@ -464,6 +540,7 @@ describe("WorkspaceStore", () => { projectName: "project-1", projectPath: "/project-1", namedWorkspacePath: "/path/1", + createdAt: new Date().toISOString(), }; const metadata2: FrontendWorkspaceMetadata = { id: "workspace-2", @@ -471,6 +548,7 @@ describe("WorkspaceStore", () => { projectName: "project-2", projectPath: "/project-2", namedWorkspacePath: "/path/2", + createdAt: new Date().toISOString(), }; // Add workspaces concurrently @@ -490,6 +568,7 @@ describe("WorkspaceStore", () => { projectName: "test-project", projectPath: "/test/project", namedWorkspacePath: "/test/project/test-workspace", + createdAt: new Date().toISOString(), }; store.addWorkspace(metadata); diff --git a/src/stores/WorkspaceStore.ts b/src/stores/WorkspaceStore.ts index 6a77b54cb..692de2c24 100644 --- a/src/stores/WorkspaceStore.ts +++ b/src/stores/WorkspaceStore.ts @@ -326,13 +326,26 @@ export class WorkspaceStore { return this.states.subscribeKey(workspaceId, listener); }; + /** + * Assert that workspace exists and return its aggregator. + * Centralized assertion for all workspace access methods. + */ + private assertGet(workspaceId: string): StreamingMessageAggregator { + const aggregator = this.aggregators.get(workspaceId); + assert(aggregator, `Workspace ${workspaceId} not found - must call addWorkspace() first`); + return aggregator; + } + /** * Get state for a specific workspace. * Lazy computation - only runs when version changes. + * + * REQUIRES: Workspace must have been added via addWorkspace() first. */ getWorkspaceState(workspaceId: string): WorkspaceState { return this.states.get(workspaceId, () => { - const aggregator = this.getOrCreateAggregator(workspaceId); + const aggregator = this.assertGet(workspaceId); + const hasMessages = aggregator.hasMessages(); const isCaughtUp = this.caughtUp.get(workspaceId) ?? false; const activeStreams = aggregator.getActiveStreams(); @@ -417,9 +430,11 @@ export class WorkspaceStore { /** * Get aggregator for a workspace (used by components that need direct access). + * + * REQUIRES: Workspace must have been added via addWorkspace() first. */ getAggregator(workspaceId: string): StreamingMessageAggregator { - return this.getOrCreateAggregator(workspaceId); + return this.assertGet(workspaceId); } /** @@ -434,10 +449,13 @@ export class WorkspaceStore { /** * Extract usage from messages (no tokenization). * Each usage entry calculated with its own model for accurate costs. + * + * REQUIRES: Workspace must have been added via addWorkspace() first. */ getWorkspaceUsage(workspaceId: string): WorkspaceUsageState { return this.usageStore.get(workspaceId, () => { - const aggregator = this.getOrCreateAggregator(workspaceId); + const aggregator = this.assertGet(workspaceId); + const messages = aggregator.getAllMessages(); // Extract usage from assistant messages @@ -737,6 +755,12 @@ export class WorkspaceStore { // Store metadata for name lookup this.workspaceMetadata.set(workspaceId, metadata); + // Backend guarantees createdAt via config.ts - this should never be undefined + assert( + metadata.createdAt, + `Workspace ${workspaceId} missing createdAt - backend contract violated` + ); + const aggregator = this.getOrCreateAggregator(workspaceId, metadata.createdAt); // Initialize recency cache and bump derived store immediately @@ -844,23 +868,24 @@ export class WorkspaceStore { // Private methods + /** + * Get or create aggregator for a workspace. + * + * REQUIRES: createdAt must be provided for new aggregators. + * Backend guarantees every workspace has createdAt via config.ts. + * + * If aggregator already exists, createdAt is optional (it was already set during creation). + */ private getOrCreateAggregator( workspaceId: string, - createdAt?: string + createdAt: string ): StreamingMessageAggregator { - // Store createdAt if provided (ensures it's never lost) - if (createdAt) { + if (!this.aggregators.has(workspaceId)) { + // Create new aggregator with required createdAt + this.aggregators.set(workspaceId, new StreamingMessageAggregator(createdAt)); this.workspaceCreatedAt.set(workspaceId, createdAt); } - if (!this.aggregators.has(workspaceId)) { - // Use stored createdAt if available, otherwise use provided value - const storedCreatedAt = this.workspaceCreatedAt.get(workspaceId); - this.aggregators.set( - workspaceId, - new StreamingMessageAggregator(storedCreatedAt ?? createdAt) - ); - } return this.aggregators.get(workspaceId)!; } @@ -873,7 +898,9 @@ export class WorkspaceStore { } private handleChatMessage(workspaceId: string, data: WorkspaceChatMessage): void { - const aggregator = this.getOrCreateAggregator(workspaceId); + // Aggregator must exist - IPC subscription happens in addWorkspace() + const aggregator = this.assertGet(workspaceId); + const isCaughtUp = this.caughtUp.get(workspaceId) ?? false; const historicalMsgs = this.historicalMessages.get(workspaceId) ?? []; diff --git a/src/utils/messages/StreamingMessageAggregator.init.test.ts b/src/utils/messages/StreamingMessageAggregator.init.test.ts index 084608bb5..2f72fec28 100644 --- a/src/utils/messages/StreamingMessageAggregator.init.test.ts +++ b/src/utils/messages/StreamingMessageAggregator.init.test.ts @@ -9,7 +9,7 @@ interface InitDisplayedMessage { describe("Init display after cleanup changes", () => { it("should display init messages correctly", () => { - const aggregator = new StreamingMessageAggregator(); + const aggregator = new StreamingMessageAggregator("2024-01-01T00:00:00.000Z"); // Simulate init start aggregator.handleMessage({ @@ -49,7 +49,7 @@ describe("Init display after cleanup changes", () => { }); it("should handle init-output without init-start (defensive)", () => { - const aggregator = new StreamingMessageAggregator(); + const aggregator = new StreamingMessageAggregator("2024-01-01T00:00:00.000Z"); // This might crash with non-null assertion if initState is null expect(() => { @@ -63,7 +63,7 @@ describe("Init display after cleanup changes", () => { }); it("should handle init-end without init-start (defensive)", () => { - const aggregator = new StreamingMessageAggregator(); + const aggregator = new StreamingMessageAggregator("2024-01-01T00:00:00.000Z"); expect(() => { aggregator.handleMessage({ diff --git a/src/utils/messages/StreamingMessageAggregator.test.ts b/src/utils/messages/StreamingMessageAggregator.test.ts index ffce06fc9..69252f2d7 100644 --- a/src/utils/messages/StreamingMessageAggregator.test.ts +++ b/src/utils/messages/StreamingMessageAggregator.test.ts @@ -1,10 +1,13 @@ import { describe, test, expect } from "bun:test"; import { StreamingMessageAggregator } from "./StreamingMessageAggregator"; +// Test helper: create aggregator with default createdAt for tests +const TEST_CREATED_AT = "2024-01-01T00:00:00.000Z"; + describe("StreamingMessageAggregator", () => { describe("init state reference stability", () => { test("should return new array reference when state changes", () => { - const aggregator = new StreamingMessageAggregator(); + const aggregator = new StreamingMessageAggregator(TEST_CREATED_AT); // Start init hook aggregator.handleMessage({ @@ -30,7 +33,7 @@ describe("StreamingMessageAggregator", () => { }); test("should return new lines array reference when init state changes", () => { - const aggregator = new StreamingMessageAggregator(); + const aggregator = new StreamingMessageAggregator(TEST_CREATED_AT); // Start init hook aggregator.handleMessage({ @@ -64,7 +67,7 @@ describe("StreamingMessageAggregator", () => { }); test("should create new init message object on each state change", () => { - const aggregator = new StreamingMessageAggregator(); + const aggregator = new StreamingMessageAggregator(TEST_CREATED_AT); // Start init hook aggregator.handleMessage({ @@ -118,7 +121,7 @@ describe("StreamingMessageAggregator", () => { }); test("should return same cached reference when state has not changed", () => { - const aggregator = new StreamingMessageAggregator(); + const aggregator = new StreamingMessageAggregator(TEST_CREATED_AT); // Start init hook aggregator.handleMessage({ diff --git a/src/utils/messages/StreamingMessageAggregator.ts b/src/utils/messages/StreamingMessageAggregator.ts index 81a2f3c09..b71c09fe1 100644 --- a/src/utils/messages/StreamingMessageAggregator.ts +++ b/src/utils/messages/StreamingMessageAggregator.ts @@ -61,16 +61,13 @@ export class StreamingMessageAggregator { } | null = null; // Workspace creation timestamp (used for recency calculation) - private readonly createdAt?: string; + // REQUIRED: Backend guarantees every workspace has createdAt via config.ts + private readonly createdAt: string; - constructor(createdAt?: string) { + constructor(createdAt: string) { this.createdAt = createdAt; - // Initialize recency immediately (ensures workspace appears at correct position before messages load) - if (createdAt) { - this.updateRecency(); - } + this.updateRecency(); } - private invalidateCache(): void { this.cachedAllMessages = null; this.cachedDisplayedMessages = null;