From e79e51107f9360160ebb009b4c0a6c577e458303 Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 24 Oct 2025 12:23:08 -0500 Subject: [PATCH 1/9] =?UTF-8?q?=F0=9F=A4=96=20Guarantee=20workspace=20crea?= =?UTF-8?q?tedAt=20at=20backend=20level?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend now ensures every workspace has createdAt timestamp via config.ts, eliminating race conditions where aggregators could be created without it. **Architecture: Fail-fast instead of defensive** - Backend guarantees createdAt for all workspaces (new, legacy, error cases) - StreamingMessageAggregator constructor requires createdAt (not optional) - WorkspaceStore asserts workspace exists before access - Clear error messages when contract violated **Changes:** - config.ts: Add createdAt to all loading paths with migration - StreamingMessageAggregator: Require createdAt in constructor, make readonly - WorkspaceStore: Simplify getOrCreateAggregator, add assertions - Tests: Update 17 test cases to match fail-fast behavior **Impact:** - Fixes workspace recency bug (new workspaces stay at top) - Removed ~30 lines of defensive logic - Fail-fast catches bugs immediately - All 741 tests passing _Generated with `cmux`_ --- src/config.ts | 24 ++- src/stores/WorkspaceStore.test.ts | 170 +++++++++++++----- src/stores/WorkspaceStore.ts | 49 +++-- .../StreamingMessageAggregator.init.test.ts | 6 +- .../StreamingMessageAggregator.test.ts | 12 +- .../messages/StreamingMessageAggregator.ts | 11 +- 6 files changed, 197 insertions(+), 75 deletions(-) diff --git a/src/config.ts b/src/config.ts index df825ae8c..22b8525e8 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 } @@ -293,6 +305,11 @@ export class Config { projectName: metadata.projectName ?? projectName, }; } + + // GUARANTEE: All workspaces must have createdAt + if (!metadata.createdAt) { + metadata.createdAt = new Date().toISOString(); + } // Migrate to config for next load workspace.id = metadata.id; @@ -312,11 +329,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 +350,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/stores/WorkspaceStore.test.ts b/src/stores/WorkspaceStore.test.ts index a10d7064b..9a689ee4f 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,74 @@ 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 +154,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 +182,7 @@ describe("WorkspaceStore", () => { projectName: "test-project", projectPath: "/test/project", namedWorkspacePath: "/test/project/test-workspace", + createdAt: new Date().toISOString(), }; store.addWorkspace(metadata); @@ -117,6 +205,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 +224,7 @@ describe("WorkspaceStore", () => { projectName: "project-1", projectPath: "/project-1", namedWorkspacePath: "/path/1", + createdAt: new Date().toISOString(), }; // Add workspace @@ -151,7 +241,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 +252,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 +290,7 @@ describe("WorkspaceStore", () => { projectName: "test-project", projectPath: "/test/project", namedWorkspacePath: "/test/project/test-workspace", + createdAt: new Date().toISOString(), }; store.addWorkspace(metadata); @@ -241,6 +335,7 @@ describe("WorkspaceStore", () => { projectName: "test-project", projectPath: "/test/project", namedWorkspacePath: "/test/project/test-workspace", + createdAt: new Date().toISOString(), }; store.addWorkspace(metadata); @@ -268,6 +363,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 +390,7 @@ describe("WorkspaceStore", () => { projectName: "test-project", projectPath: "/test/project", namedWorkspacePath: "/test/project/test-workspace", + createdAt: new Date().toISOString(), }; store.addWorkspace(metadata); @@ -330,6 +429,7 @@ describe("WorkspaceStore", () => { projectName: "test-project", projectPath: "/test/project", namedWorkspacePath: "/test/project/test-workspace", + createdAt: new Date().toISOString(), }; store.addWorkspace(metadata); @@ -360,27 +460,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", async () => { + 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 recency2 = store.getWorkspaceRecency(); - expect(recency1).not.toBe(recency2); // Cache should be invalidated + const recency = store.getWorkspaceRecency(); + + // Recency should be based on createdAt + expect(recency["test-workspace"]).toBe(new Date(createdAt).getTime()); }); it("maintains cache when no changes occur", () => { @@ -390,6 +485,7 @@ describe("WorkspaceStore", () => { projectName: "test-project", projectPath: "/test/project", namedWorkspacePath: "/test/project/test-workspace", + createdAt: new Date().toISOString(), }; store.addWorkspace(metadata); @@ -411,50 +507,31 @@ describe("WorkspaceStore", () => { }); describe("race conditions", () => { - it("handles IPC message for removed workspace gracefully", async () => { + it("properly cleans up workspace on removal", async () => { 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(); - - // 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 workspace is completely removed + allStates = store.getAllStates(); + expect(allStates.size).toBe(0); + + // Verify aggregator is gone + expect(store.getAggregator).toBeDefined(); + expect(() => store.getAggregator("test-workspace")).toThrow(/Workspace test-workspace not found/); }); it("handles concurrent workspace additions", () => { @@ -464,6 +541,7 @@ describe("WorkspaceStore", () => { projectName: "project-1", projectPath: "/project-1", namedWorkspacePath: "/path/1", + createdAt: new Date().toISOString(), }; const metadata2: FrontendWorkspaceMetadata = { id: "workspace-2", @@ -471,6 +549,7 @@ describe("WorkspaceStore", () => { projectName: "project-2", projectPath: "/project-2", namedWorkspacePath: "/path/2", + createdAt: new Date().toISOString(), }; // Add workspaces concurrently @@ -490,6 +569,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..264665f57 100644 --- a/src/stores/WorkspaceStore.ts +++ b/src/stores/WorkspaceStore.ts @@ -329,10 +329,14 @@ export class WorkspaceStore { /** * 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.aggregators.get(workspaceId); + assert(aggregator, `Workspace ${workspaceId} not found - must call addWorkspace() first`); + const hasMessages = aggregator.hasMessages(); const isCaughtUp = this.caughtUp.get(workspaceId) ?? false; const activeStreams = aggregator.getActiveStreams(); @@ -417,9 +421,13 @@ 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); + const aggregator = this.aggregators.get(workspaceId); + assert(aggregator, `Workspace ${workspaceId} not found - must call addWorkspace() first`); + return aggregator; } /** @@ -434,10 +442,14 @@ 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.aggregators.get(workspaceId); + assert(aggregator, `Workspace ${workspaceId} not found - must call addWorkspace() first`); + const messages = aggregator.getAllMessages(); // Extract usage from assistant messages @@ -737,6 +749,9 @@ 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 +859,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 +889,10 @@ 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.aggregators.get(workspaceId); + assert(aggregator, `Workspace ${workspaceId} not found - IPC message arrived before addWorkspace()`); + 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..6c479911d 100644 --- a/src/utils/messages/StreamingMessageAggregator.test.ts +++ b/src/utils/messages/StreamingMessageAggregator.test.ts @@ -1,10 +1,14 @@ 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 +34,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 +68,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 +122,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; From 3d2b8b8c21f350543c76759b845a2fbb00522ce4 Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 24 Oct 2025 12:51:28 -0500 Subject: [PATCH 2/9] Fix: Gate selectedWorkspace behind metadata loading Problem: WorkspaceStore methods assert that addWorkspace() was called, but selectedWorkspace could be restored from localStorage BEFORE metadata loads and syncWorkspaces() runs. This caused assertions to fire on app startup: 'Workspace X not found - must call addWorkspace() first' Solution: Two-stage workspace selection 1. persistedSelection - restored from localStorage immediately (internal) 2. selectedWorkspace - null until metadata loads, then restored This ensures WorkspaceStore.syncWorkspaces() completes before any component can access workspace functionality. Also: Added note to AGENTS.md to never kill running cmux process --- docs/AGENTS.md | 4 ++++ src/App.tsx | 52 +++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/docs/AGENTS.md b/docs/AGENTS.md index 62184dbcd..9f3e750fb 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -178,6 +178,10 @@ 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.tsx b/src/App.tsx index 59bf74c6f..493cd996e 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -34,10 +34,30 @@ import { useTelemetry } from "./hooks/useTelemetry"; const THINKING_LEVELS: ThinkingLevel[] = ["off", "low", "medium", "high"]; function AppInner() { - const [selectedWorkspace, setSelectedWorkspace] = usePersistedState( + // CRITICAL ARCHITECTURE: Two-stage workspace selection + // ======================================================= + // Problem: Components using selectedWorkspace call WorkspaceStore methods that require + // addWorkspace() to be called first. But localStorage can restore selectedWorkspace + // BEFORE metadata loads and syncWorkspaces() runs. + // + // Solution: Gate selectedWorkspace behind metadata loading: + // 1. persistedSelection - restored from localStorage immediately (internal only) + // 2. selectedWorkspace - null until metadata loads, then restored from persistedSelection + // + // This ensures WorkspaceStore.syncWorkspaces() runs before any component can access + // workspace-related functionality. + + // Stage 1: Persisted value (internal, not exposed to components until ready) + const [persistedSelection, setPersistedSelection] = usePersistedState( "selectedWorkspace", null ); + + // Stage 2: Actual selectedWorkspace (null until metadata loads) + const [selectedWorkspace, setSelectedWorkspaceInternal] = useState( + null + ); + const [workspaceModalOpen, setWorkspaceModalOpen] = useState(false); const [workspaceModalProject, setWorkspaceModalProject] = useState(null); const [workspaceModalProjectName, setWorkspaceModalProjectName] = useState(""); @@ -59,10 +79,10 @@ function AppInner() { // Telemetry tracking const telemetry = useTelemetry(); - // Wrapper for setSelectedWorkspace that tracks telemetry - const handleWorkspaceSwitch = useCallback( + // Wrapper that persists changes and tracks telemetry + const setSelectedWorkspace = useCallback( (newWorkspace: WorkspaceSelection | null) => { - console.debug("[App] handleWorkspaceSwitch called", { + console.debug("[App] setSelectedWorkspace called", { from: selectedWorkspace?.workspaceId, to: newWorkspace?.workspaceId, }); @@ -76,9 +96,20 @@ function AppInner() { console.debug("[App] Calling telemetry.workspaceSwitched"); telemetry.workspaceSwitched(selectedWorkspace.workspaceId, newWorkspace.workspaceId); } + + // Update both internal state and persisted value + setSelectedWorkspaceInternal(newWorkspace); + setPersistedSelection(newWorkspace); + }, + [selectedWorkspace, setPersistedSelection, telemetry] + ); + + // Wrapper for setSelectedWorkspace that tracks telemetry (kept for compatibility) + const handleWorkspaceSwitch = useCallback( + (newWorkspace: WorkspaceSelection | null) => { setSelectedWorkspace(newWorkspace); }, - [selectedWorkspace, setSelectedWorkspace, telemetry] + [setSelectedWorkspace] ); // Use custom hooks for project and workspace management @@ -123,6 +154,17 @@ function AppInner() { } }, [workspaceMetadata, gitStatusStore]); + // Restore persisted selection once metadata is loaded and stores are synced + // This ensures WorkspaceStore.addWorkspace() is called before any workspace access + useEffect(() => { + if (metadataLoading) return; + + // Restore persisted selection if it still exists in metadata + if (persistedSelection && workspaceMetadata.has(persistedSelection.workspaceId)) { + setSelectedWorkspaceInternal(persistedSelection); + } + }, [metadataLoading, persistedSelection, workspaceMetadata]); + // Track last-read timestamps for unread indicators const { lastReadTimestamps, onToggleUnread } = useUnreadTracking(selectedWorkspace); From b0a899e7ace590d379131d5ed76544d364dacd2c Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 24 Oct 2025 12:55:53 -0500 Subject: [PATCH 3/9] Refactor: Clean architecture - gate workspace UI behind metadata loading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem: Previous fix used two-stage selection (persistedSelection → selectedWorkspace) with scattered metadataLoading guards. Complex and still allowed workspace-dependent code to run before stores synced. Solution: Show LoadingScreen until metadata loads, THEN render all workspace UI. Single point of control, impossible to access workspaces before ready. Changes: - Created LoadingScreen component (simple spinner) - Early return in AppInner() if metadataLoading - Removed two-stage selection complexity - Removed all scattered metadataLoading guards from effects - Simplified handleWorkspaceSwitch Benefits: ✅ Single point of control vs scattered guards ✅ TypeScript/runtime guarantee no workspace access before ready ✅ Simpler mental model: Loading → Ready, no intermediate states ✅ Cleaner code: Net -27 lines Net: +34 insertions, -61 deletions (-27 lines) --- docs/AGENTS.md | 2 - src/App.tsx | 88 ++++++------------- src/components/LoadingScreen.tsx | 10 +++ src/config.ts | 10 +-- src/stores/WorkspaceStore.test.ts | 35 ++++---- src/stores/WorkspaceStore.ts | 26 +++--- .../StreamingMessageAggregator.test.ts | 1 - 7 files changed, 75 insertions(+), 97 deletions(-) create mode 100644 src/components/LoadingScreen.tsx diff --git a/docs/AGENTS.md b/docs/AGENTS.md index 9f3e750fb..55d8db0cd 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -180,8 +180,6 @@ This project uses **Make** as the primary build orchestrator. See `Makefile` for **⚠️ 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.tsx b/src/App.tsx index 493cd996e..574192ea8 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -4,6 +4,7 @@ import type { ProjectConfig } from "./config"; import type { WorkspaceSelection } from "./components/ProjectSidebar"; import type { FrontendWorkspaceMetadata } from "./types/workspace"; import { LeftSidebar } from "./components/LeftSidebar"; +import { LoadingScreen } from "./components/LoadingScreen"; import NewWorkspaceModal from "./components/NewWorkspaceModal"; import { DirectorySelectModal } from "./components/DirectorySelectModal"; import { AIView } from "./components/AIView"; @@ -34,30 +35,13 @@ import { useTelemetry } from "./hooks/useTelemetry"; const THINKING_LEVELS: ThinkingLevel[] = ["off", "low", "medium", "high"]; function AppInner() { - // CRITICAL ARCHITECTURE: Two-stage workspace selection - // ======================================================= - // Problem: Components using selectedWorkspace call WorkspaceStore methods that require - // addWorkspace() to be called first. But localStorage can restore selectedWorkspace - // BEFORE metadata loads and syncWorkspaces() runs. - // - // Solution: Gate selectedWorkspace behind metadata loading: - // 1. persistedSelection - restored from localStorage immediately (internal only) - // 2. selectedWorkspace - null until metadata loads, then restored from persistedSelection - // - // This ensures WorkspaceStore.syncWorkspaces() runs before any component can access - // workspace-related functionality. - - // Stage 1: Persisted value (internal, not exposed to components until ready) - const [persistedSelection, setPersistedSelection] = usePersistedState( + // Workspace selection - restored from localStorage immediately, + // but entire UI is gated behind metadata loading (see early return below) + const [selectedWorkspace, setSelectedWorkspace] = usePersistedState( "selectedWorkspace", null ); - // Stage 2: Actual selectedWorkspace (null until metadata loads) - const [selectedWorkspace, setSelectedWorkspaceInternal] = useState( - null - ); - const [workspaceModalOpen, setWorkspaceModalOpen] = useState(false); const [workspaceModalProject, setWorkspaceModalProject] = useState(null); const [workspaceModalProjectName, setWorkspaceModalProjectName] = useState(""); @@ -79,10 +63,10 @@ function AppInner() { // Telemetry tracking const telemetry = useTelemetry(); - // Wrapper that persists changes and tracks telemetry - const setSelectedWorkspace = useCallback( + // Wrapper for setSelectedWorkspace that tracks telemetry + const handleWorkspaceSwitch = useCallback( (newWorkspace: WorkspaceSelection | null) => { - console.debug("[App] setSelectedWorkspace called", { + console.debug("[App] handleWorkspaceSwitch called", { from: selectedWorkspace?.workspaceId, to: newWorkspace?.workspaceId, }); @@ -97,19 +81,9 @@ function AppInner() { telemetry.workspaceSwitched(selectedWorkspace.workspaceId, newWorkspace.workspaceId); } - // Update both internal state and persisted value - setSelectedWorkspaceInternal(newWorkspace); - setPersistedSelection(newWorkspace); - }, - [selectedWorkspace, setPersistedSelection, telemetry] - ); - - // Wrapper for setSelectedWorkspace that tracks telemetry (kept for compatibility) - const handleWorkspaceSwitch = useCallback( - (newWorkspace: WorkspaceSelection | null) => { setSelectedWorkspace(newWorkspace); }, - [setSelectedWorkspace] + [selectedWorkspace, setSelectedWorkspace, telemetry] ); // Use custom hooks for project and workspace management @@ -136,34 +110,28 @@ function AppInner() { onSelectedWorkspaceUpdate: setSelectedWorkspace, }); - // NEW: Sync workspace metadata with the stores + // Sync workspace metadata with the stores BEFORE rendering workspace UI const workspaceStore = useWorkspaceStoreRaw(); const gitStatusStore = useGitStatusStoreRaw(); useEffect(() => { - // Only sync when metadata has actually loaded (not empty initial state) - if (workspaceMetadata.size > 0) { + if (!metadataLoading) { workspaceStore.syncWorkspaces(workspaceMetadata); - } - }, [workspaceMetadata, workspaceStore]); - - useEffect(() => { - // Only sync when metadata has actually loaded (not empty initial state) - if (workspaceMetadata.size > 0) { gitStatusStore.syncWorkspaces(workspaceMetadata); } - }, [workspaceMetadata, gitStatusStore]); + }, [metadataLoading, workspaceMetadata, workspaceStore, gitStatusStore]); - // Restore persisted selection once metadata is loaded and stores are synced - // This ensures WorkspaceStore.addWorkspace() is called before any workspace access + // Validate selectedWorkspace when metadata changes + // Clear selection if workspace was deleted useEffect(() => { - if (metadataLoading) return; - - // Restore persisted selection if it still exists in metadata - if (persistedSelection && workspaceMetadata.has(persistedSelection.workspaceId)) { - setSelectedWorkspaceInternal(persistedSelection); + if ( + !metadataLoading && + selectedWorkspace && + !workspaceMetadata.has(selectedWorkspace.workspaceId) + ) { + setSelectedWorkspace(null); } - }, [metadataLoading, persistedSelection, workspaceMetadata]); + }, [metadataLoading, selectedWorkspace, workspaceMetadata, setSelectedWorkspace]); // Track last-read timestamps for unread indicators const { lastReadTimestamps, onToggleUnread } = useUnreadTracking(selectedWorkspace); @@ -205,9 +173,6 @@ function AppInner() { // 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)); @@ -227,13 +192,10 @@ function AppInner() { } setHasRestoredFromHash(true); - }, [metadataLoading, workspaceMetadata, hasRestoredFromHash, setSelectedWorkspace]); + }, [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); @@ -257,7 +219,7 @@ function AppInner() { }); } } - }, [metadataLoading, selectedWorkspace, workspaceMetadata, setSelectedWorkspace]); + }, [selectedWorkspace, workspaceMetadata, setSelectedWorkspace]); const openWorkspaceInTerminal = useCallback( (workspaceId: string) => { @@ -702,6 +664,12 @@ function AppInner() { ); }, [projects, setSelectedWorkspace, setWorkspaceMetadata]); + // CRITICAL: Don't render workspace UI until metadata loads and stores are synced + // This ensures WorkspaceStore.addWorkspace() is called before any component accesses workspaces + if (metadataLoading) { + return ; + } + return ( <>
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 22b8525e8..2a8ab46f1 100644 --- a/src/config.ts +++ b/src/config.ts @@ -275,13 +275,13 @@ export class Config { // 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 } @@ -305,11 +305,9 @@ export class Config { projectName: metadata.projectName ?? projectName, }; } - + // GUARANTEE: All workspaces must have createdAt - if (!metadata.createdAt) { - metadata.createdAt = new Date().toISOString(); - } + metadata.createdAt ??= new Date().toISOString(); // Migrate to config for next load workspace.id = metadata.id; diff --git a/src/stores/WorkspaceStore.test.ts b/src/stores/WorkspaceStore.test.ts index 9a689ee4f..c29f73205 100644 --- a/src/stores/WorkspaceStore.test.ts +++ b/src/stores/WorkspaceStore.test.ts @@ -89,14 +89,14 @@ describe("WorkspaceStore", () => { // 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()); @@ -116,32 +116,30 @@ describe("WorkspaceStore", () => { // 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)); - + 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(); @@ -460,7 +458,7 @@ describe("WorkspaceStore", () => { expect(states1).not.toBe(states2); // Cache should be invalidated }); - it("maintains recency based on createdAt for new workspaces", 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", @@ -473,7 +471,7 @@ describe("WorkspaceStore", () => { store.addWorkspace(metadata); const recency = store.getWorkspaceRecency(); - + // Recency should be based on createdAt expect(recency["test-workspace"]).toBe(new Date(createdAt).getTime()); }); @@ -507,7 +505,7 @@ describe("WorkspaceStore", () => { }); describe("race conditions", () => { - it("properly cleans up workspace on removal", async () => { + it("properly cleans up workspace on removal", () => { const metadata: FrontendWorkspaceMetadata = { id: "test-workspace", name: "test-workspace", @@ -528,10 +526,11 @@ describe("WorkspaceStore", () => { // Verify workspace is completely removed allStates = store.getAllStates(); expect(allStates.size).toBe(0); - + // Verify aggregator is gone - expect(store.getAggregator).toBeDefined(); - expect(() => store.getAggregator("test-workspace")).toThrow(/Workspace test-workspace not found/); + expect(() => store.getAggregator("test-workspace")).toThrow( + /Workspace test-workspace not found/ + ); }); it("handles concurrent workspace additions", () => { diff --git a/src/stores/WorkspaceStore.ts b/src/stores/WorkspaceStore.ts index 264665f57..bc6526719 100644 --- a/src/stores/WorkspaceStore.ts +++ b/src/stores/WorkspaceStore.ts @@ -329,14 +329,14 @@ export class WorkspaceStore { /** * 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.aggregators.get(workspaceId); assert(aggregator, `Workspace ${workspaceId} not found - must call addWorkspace() first`); - + const hasMessages = aggregator.hasMessages(); const isCaughtUp = this.caughtUp.get(workspaceId) ?? false; const activeStreams = aggregator.getActiveStreams(); @@ -421,7 +421,7 @@ 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 { @@ -442,14 +442,14 @@ 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.aggregators.get(workspaceId); assert(aggregator, `Workspace ${workspaceId} not found - must call addWorkspace() first`); - + const messages = aggregator.getAllMessages(); // Extract usage from assistant messages @@ -750,7 +750,10 @@ export class WorkspaceStore { 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`); + assert( + metadata.createdAt, + `Workspace ${workspaceId} missing createdAt - backend contract violated` + ); const aggregator = this.getOrCreateAggregator(workspaceId, metadata.createdAt); @@ -861,10 +864,10 @@ export class WorkspaceStore { /** * 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( @@ -891,8 +894,11 @@ export class WorkspaceStore { private handleChatMessage(workspaceId: string, data: WorkspaceChatMessage): void { // Aggregator must exist - IPC subscription happens in addWorkspace() const aggregator = this.aggregators.get(workspaceId); - assert(aggregator, `Workspace ${workspaceId} not found - IPC message arrived before addWorkspace()`); - + assert( + aggregator, + `Workspace ${workspaceId} not found - IPC message arrived before addWorkspace()` + ); + const isCaughtUp = this.caughtUp.get(workspaceId) ?? false; const historicalMsgs = this.historicalMessages.get(workspaceId) ?? []; diff --git a/src/utils/messages/StreamingMessageAggregator.test.ts b/src/utils/messages/StreamingMessageAggregator.test.ts index 6c479911d..69252f2d7 100644 --- a/src/utils/messages/StreamingMessageAggregator.test.ts +++ b/src/utils/messages/StreamingMessageAggregator.test.ts @@ -1,7 +1,6 @@ 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"; From b456896f6753b78b33e0bb80d4f51e160e037baf Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 24 Oct 2025 13:10:03 -0500 Subject: [PATCH 4/9] Refactor: Deduplicate assertions with assertGet helper Consolidate 4 duplicate assertions into single private assertGet() method. Cleaner and ensures consistent error messages across all workspace access. --- src/stores/WorkspaceStore.ts | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/stores/WorkspaceStore.ts b/src/stores/WorkspaceStore.ts index bc6526719..692de2c24 100644 --- a/src/stores/WorkspaceStore.ts +++ b/src/stores/WorkspaceStore.ts @@ -326,6 +326,16 @@ 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. @@ -334,8 +344,7 @@ export class WorkspaceStore { */ getWorkspaceState(workspaceId: string): WorkspaceState { return this.states.get(workspaceId, () => { - const aggregator = this.aggregators.get(workspaceId); - assert(aggregator, `Workspace ${workspaceId} not found - must call addWorkspace() first`); + const aggregator = this.assertGet(workspaceId); const hasMessages = aggregator.hasMessages(); const isCaughtUp = this.caughtUp.get(workspaceId) ?? false; @@ -425,9 +434,7 @@ export class WorkspaceStore { * REQUIRES: Workspace must have been added via addWorkspace() first. */ getAggregator(workspaceId: string): StreamingMessageAggregator { - const aggregator = this.aggregators.get(workspaceId); - assert(aggregator, `Workspace ${workspaceId} not found - must call addWorkspace() first`); - return aggregator; + return this.assertGet(workspaceId); } /** @@ -447,8 +454,7 @@ export class WorkspaceStore { */ getWorkspaceUsage(workspaceId: string): WorkspaceUsageState { return this.usageStore.get(workspaceId, () => { - const aggregator = this.aggregators.get(workspaceId); - assert(aggregator, `Workspace ${workspaceId} not found - must call addWorkspace() first`); + const aggregator = this.assertGet(workspaceId); const messages = aggregator.getAllMessages(); @@ -893,11 +899,7 @@ export class WorkspaceStore { private handleChatMessage(workspaceId: string, data: WorkspaceChatMessage): void { // Aggregator must exist - IPC subscription happens in addWorkspace() - const aggregator = this.aggregators.get(workspaceId); - assert( - aggregator, - `Workspace ${workspaceId} not found - IPC message arrived before addWorkspace()` - ); + const aggregator = this.assertGet(workspaceId); const isCaughtUp = this.caughtUp.get(workspaceId) ?? false; const historicalMsgs = this.historicalMessages.get(workspaceId) ?? []; From 312009635814f34c934325bdff8af22f3d7b5e2a Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 24 Oct 2025 13:49:09 -0500 Subject: [PATCH 5/9] Add defensive createdAt insertion on frontend Add ensureCreatedAt() helper that defaults to 2025-01-01 if backend violates the createdAt guarantee. Applied to all metadata loading paths: - workspace.list() - workspace.onMetadata() updates - workspace.getInfo() after rename - workspace fork switch event This prevents crashes if backend contract is violated. --- src/App.tsx | 8 ++++++++ src/hooks/useWorkspaceManagement.ts | 17 +++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/App.tsx b/src/App.tsx index 574192ea8..9d94c98c6 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -639,6 +639,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) => { 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, From b692973d666f47948fc5f68726ab045518b5705f Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 24 Oct 2025 13:52:12 -0500 Subject: [PATCH 6/9] Fix: Add storesSynced state to prevent render before sync Problem: syncWorkspaces runs in useEffect AFTER render, but sidebar components access the store DURING render, causing assertion failures. Solution: Add storesSynced state that only becomes true after syncWorkspaces completes. Gate UI rendering on both metadataLoading AND storesSynced to ensure stores are ready before any component tries to access them. --- src/App.tsx | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/App.tsx b/src/App.tsx index 9d94c98c6..026377b28 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -114,24 +114,26 @@ function AppInner() { const workspaceStore = useWorkspaceStoreRaw(); const gitStatusStore = useGitStatusStoreRaw(); + // Track whether stores have been synced (separate from metadata loading) + const [storesSynced, setStoresSynced] = useState(false); + useEffect(() => { if (!metadataLoading) { workspaceStore.syncWorkspaces(workspaceMetadata); gitStatusStore.syncWorkspaces(workspaceMetadata); + setStoresSynced(true); + } else { + setStoresSynced(false); } }, [metadataLoading, workspaceMetadata, workspaceStore, gitStatusStore]); // Validate selectedWorkspace when metadata changes // Clear selection if workspace was deleted useEffect(() => { - if ( - !metadataLoading && - selectedWorkspace && - !workspaceMetadata.has(selectedWorkspace.workspaceId) - ) { + if (storesSynced && selectedWorkspace && !workspaceMetadata.has(selectedWorkspace.workspaceId)) { setSelectedWorkspace(null); } - }, [metadataLoading, selectedWorkspace, workspaceMetadata, setSelectedWorkspace]); + }, [storesSynced, selectedWorkspace, workspaceMetadata, setSelectedWorkspace]); // Track last-read timestamps for unread indicators const { lastReadTimestamps, onToggleUnread } = useUnreadTracking(selectedWorkspace); @@ -672,9 +674,9 @@ function AppInner() { ); }, [projects, setSelectedWorkspace, setWorkspaceMetadata]); - // CRITICAL: Don't render workspace UI until metadata loads and stores are synced + // CRITICAL: Don't render workspace UI until metadata loads AND stores are synced // This ensures WorkspaceStore.addWorkspace() is called before any component accesses workspaces - if (metadataLoading) { + if (metadataLoading || !storesSynced) { return ; } From d1d4d1e255edcf1226f099491a4533a79e6bef81 Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 24 Oct 2025 14:06:41 -0500 Subject: [PATCH 7/9] =?UTF-8?q?=F0=9F=A4=96=20Simplify=20initialization=20?= =?UTF-8?q?with=20AppLoader=20wrapper=20component?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Refactored app initialization to use a dedicated AppLoader wrapper that handles all loading, syncing, and restoration before rendering the main App. **Problem:** - App.tsx had scattered `storesSynced` gating logic (9 references) - 3 effects with `if (!storesSynced) return` guards - Complex dependency arrays and execution ordering concerns - Early return with LoadingScreen created confusing flow **Solution:** - Created `AppLoader` component that handles: 1. Load workspace metadata and projects 2. Sync stores with loaded data 3. Restore workspace from URL hash (if present) 4. Only render App when everything is ready - App.tsx now: - Accepts all state as props (no longer calls management hooks) - Assumes stores are always synced (no guards needed) - Simpler effects without conditional execution - Clearer separation of concerns **Benefits:** - Explicit loading boundary at component tree level - All initialization logic in one place - App.tsx is 48 lines shorter - No more race conditions between effects - Similar to Suspense pattern but simpler **Changes:** - New: `src/components/AppLoader.tsx` (115 lines) - Modified: `src/App.tsx` (-75 lines, removed init logic) - Modified: `src/main.tsx` (render AppLoader instead of App) - Net: +40 lines **Testing:** - All 765 tests passing - Build successful - Type checking clean _Generated with `cmux`_ --- src/App.tsx | 148 ++++++++++++----------------------- src/components/AppLoader.tsx | 115 +++++++++++++++++++++++++++ src/main.tsx | 4 +- 3 files changed, 167 insertions(+), 100 deletions(-) create mode 100644 src/components/AppLoader.tsx diff --git a/src/App.tsx b/src/App.tsx index 026377b28..ff741f180 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1,23 +1,19 @@ -import { useState, useEffect, useCallback, useRef } from "react"; +import { useState, useEffect, useCallback, useRef, type Dispatch, type SetStateAction } from "react"; import "./styles/globals.css"; import type { ProjectConfig } from "./config"; import type { WorkspaceSelection } from "./components/ProjectSidebar"; import type { FrontendWorkspaceMetadata } from "./types/workspace"; import { LeftSidebar } from "./components/LeftSidebar"; -import { LoadingScreen } from "./components/LoadingScreen"; import NewWorkspaceModal from "./components/NewWorkspaceModal"; import { DirectorySelectModal } from "./components/DirectorySelectModal"; 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,13 +30,48 @@ import { useTelemetry } from "./hooks/useTelemetry"; const THINKING_LEVELS: ThinkingLevel[] = ["off", "low", "medium", "high"]; -function AppInner() { - // Workspace selection - restored from localStorage immediately, - // but entire UI is gated behind metadata loading (see early return below) - const [selectedWorkspace, setSelectedWorkspace] = usePersistedState( - "selectedWorkspace", - null - ); +interface AppInnerProps { + projects: Map; + setProjects: Dispatch>>; + addProject: () => Promise; + removeProject: (path: string) => Promise; + 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 }>; + selectedWorkspace: WorkspaceSelection | null; + setSelectedWorkspace: (workspace: WorkspaceSelection | null) => void; +} + +function AppInner({ + projects, + setProjects, + addProject, + removeProject, + workspaceMetadata, + setWorkspaceMetadata, + createWorkspace, + removeWorkspace, + renameWorkspace, + selectedWorkspace, + setSelectedWorkspace, +}: AppInnerProps) { const [workspaceModalOpen, setWorkspaceModalOpen] = useState(false); const [workspaceModalProject, setWorkspaceModalProject] = useState(null); @@ -63,21 +94,18 @@ 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); } @@ -86,54 +114,13 @@ function AppInner() { [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, - }); - - // Sync workspace metadata with the stores BEFORE rendering workspace UI - const workspaceStore = useWorkspaceStoreRaw(); - const gitStatusStore = useGitStatusStoreRaw(); - - // Track whether stores have been synced (separate from metadata loading) - const [storesSynced, setStoresSynced] = useState(false); - - useEffect(() => { - if (!metadataLoading) { - workspaceStore.syncWorkspaces(workspaceMetadata); - gitStatusStore.syncWorkspaces(workspaceMetadata); - setStoresSynced(true); - } else { - setStoresSynced(false); - } - }, [metadataLoading, workspaceMetadata, workspaceStore, gitStatusStore]); - // Validate selectedWorkspace when metadata changes // Clear selection if workspace was deleted useEffect(() => { - if (storesSynced && selectedWorkspace && !workspaceMetadata.has(selectedWorkspace.workspaceId)) { + if (selectedWorkspace && !workspaceMetadata.has(selectedWorkspace.workspaceId)) { setSelectedWorkspace(null); } - }, [storesSynced, selectedWorkspace, workspaceMetadata, setSelectedWorkspace]); + }, [selectedWorkspace, workspaceMetadata, setSelectedWorkspace]); // Track last-read timestamps for unread indicators const { lastReadTimestamps, onToggleUnread } = useUnreadTracking(selectedWorkspace); @@ -167,35 +154,6 @@ 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; - - 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); - }, [workspaceMetadata, hasRestoredFromHash, setSelectedWorkspace]); - // Validate selected workspace exists and has all required fields useEffect(() => { if (selectedWorkspace) { @@ -674,12 +632,6 @@ function AppInner() { ); }, [projects, setSelectedWorkspace, setWorkspaceMetadata]); - // CRITICAL: Don't render workspace UI until metadata loads AND stores are synced - // This ensures WorkspaceStore.addWorkspace() is called before any component accesses workspaces - if (metadataLoading || !storesSynced) { - return ; - } - return ( <>
@@ -766,10 +718,10 @@ function AppInner() { ); } -function App() { +function App(props: AppInnerProps) { return ( - + ); } diff --git a/src/components/AppLoader.tsx b/src/components/AppLoader.tsx new file mode 100644 index 000000000..fd8df769e --- /dev/null +++ b/src/components/AppLoader.tsx @@ -0,0 +1,115 @@ +import { useState, useEffect } from "react"; +import App from "../App"; +import { LoadingScreen } from "./LoadingScreen"; +import { useProjectManagement } from "../hooks/useProjectManagement"; +import { useWorkspaceManagement } from "../hooks/useWorkspaceManagement"; +import { useWorkspaceStoreRaw } from "../stores/WorkspaceStore"; +import { useGitStatusStoreRaw } from "../stores/GitStatusStore"; +import { usePersistedState } from "../hooks/usePersistedState"; +import type { WorkspaceSelection } from "./ProjectSidebar"; + +/** + * AppLoader handles all initialization before rendering the main App: + * 1. Load workspace metadata and projects + * 2. Sync stores with loaded data + * 3. Restore workspace selection from URL hash (if present) + * 4. Only render App when everything is ready + * + * This ensures App.tsx can assume stores are always synced and removes + * the need for conditional guards in effects. + */ +export function AppLoader() { + // Workspace selection - restored from localStorage immediately + const [selectedWorkspace, setSelectedWorkspace] = usePersistedState( + "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 + return ( + + ); +} + 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( - + ); From 3dde8730ca3fb0eabcacfc6175b6fe04b8b22fe3 Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 24 Oct 2025 14:11:33 -0500 Subject: [PATCH 8/9] Fix: Formatting, unused var, and update storybook stories --- src/App.stories.tsx | 10 +++++----- src/App.tsx | 12 +++++++++--- src/components/AppLoader.tsx | 8 ++++++-- 3 files changed, 20 insertions(+), 10 deletions(-) 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 ff741f180..c2896116e 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1,4 +1,11 @@ -import { useState, useEffect, useCallback, useRef, type Dispatch, type SetStateAction } from "react"; +import { + useState, + useEffect, + useCallback, + useRef, + type Dispatch, + type SetStateAction, +} from "react"; import "./styles/globals.css"; import type { ProjectConfig } from "./config"; import type { WorkspaceSelection } from "./components/ProjectSidebar"; @@ -61,7 +68,7 @@ interface AppInnerProps { function AppInner({ projects, - setProjects, + setProjects: _setProjects, addProject, removeProject, workspaceMetadata, @@ -72,7 +79,6 @@ function AppInner({ selectedWorkspace, setSelectedWorkspace, }: AppInnerProps) { - const [workspaceModalOpen, setWorkspaceModalOpen] = useState(false); const [workspaceModalProject, setWorkspaceModalProject] = useState(null); const [workspaceModalProjectName, setWorkspaceModalProjectName] = useState(""); diff --git a/src/components/AppLoader.tsx b/src/components/AppLoader.tsx index fd8df769e..26aa1e4f9 100644 --- a/src/components/AppLoader.tsx +++ b/src/components/AppLoader.tsx @@ -88,7 +88,12 @@ export function AppLoader() { } setHasRestoredFromHash(true); - }, [storesSynced, workspaceManagement.workspaceMetadata, hasRestoredFromHash, setSelectedWorkspace]); + }, [ + storesSynced, + workspaceManagement.workspaceMetadata, + hasRestoredFromHash, + setSelectedWorkspace, + ]); // Show loading screen until stores are synced if (workspaceManagement.loading || !storesSynced) { @@ -112,4 +117,3 @@ export function AppLoader() { /> ); } - From 90ddc4e9647f008c052621453d0989d487689794 Mon Sep 17 00:00:00 2001 From: Ammar Date: Fri, 24 Oct 2025 22:07:41 -0500 Subject: [PATCH 9/9] =?UTF-8?q?=F0=9F=A4=96=20Eliminate=20prop=20drilling?= =?UTF-8?q?=20with=20AppContext?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace 11-prop interface (AppInnerProps) with React Context pattern: - Created AppContext to provide app-level state (projects, workspaces, selection) - Updated AppLoader to wrap App with AppProvider - Refactored App.tsx to use useApp() hook instead of props - Updated LeftSidebar to consume context directly Benefits: - Decoupling: Components access data directly without parent involvement - Scalability: Adding state touches 2 files instead of 3+ - Consistency: Matches existing patterns (ModeContext, ThinkingContext) - Readability: No verbose prop interfaces, clear separation of concerns Net: +96 insertions, -74 deletions --- src/App.tsx | 76 ++++++++-------------------------- src/components/AppLoader.tsx | 9 ++-- src/components/LeftSidebar.tsx | 24 +++++------ src/contexts/AppContext.tsx | 61 +++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 74 deletions(-) create mode 100644 src/contexts/AppContext.tsx diff --git a/src/App.tsx b/src/App.tsx index c2896116e..98918d7a1 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1,13 +1,6 @@ -import { - useState, - useEffect, - useCallback, - useRef, - type Dispatch, - type SetStateAction, -} from "react"; +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"; @@ -37,48 +30,20 @@ import { useTelemetry } from "./hooks/useTelemetry"; const THINKING_LEVELS: ThinkingLevel[] = ["off", "low", "medium", "high"]; -interface AppInnerProps { - projects: Map; - setProjects: Dispatch>>; - addProject: () => Promise; - removeProject: (path: string) => Promise; - 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 }>; - selectedWorkspace: WorkspaceSelection | null; - setSelectedWorkspace: (workspace: WorkspaceSelection | null) => void; -} - -function AppInner({ - projects, - setProjects: _setProjects, - addProject, - removeProject, - workspaceMetadata, - setWorkspaceMetadata, - createWorkspace, - removeWorkspace, - renameWorkspace, - selectedWorkspace, - setSelectedWorkspace, -}: AppInnerProps) { +function AppInner() { + // 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(""); @@ -642,15 +607,10 @@ function AppInner({ <>
- + ); } diff --git a/src/components/AppLoader.tsx b/src/components/AppLoader.tsx index 26aa1e4f9..739eee2c7 100644 --- a/src/components/AppLoader.tsx +++ b/src/components/AppLoader.tsx @@ -7,6 +7,7 @@ import { useWorkspaceStoreRaw } from "../stores/WorkspaceStore"; import { useGitStatusStoreRaw } from "../stores/GitStatusStore"; import { usePersistedState } from "../hooks/usePersistedState"; import type { WorkspaceSelection } from "./ProjectSidebar"; +import { AppProvider } from "../contexts/AppContext"; /** * AppLoader handles all initialization before rendering the main App: @@ -100,9 +101,9 @@ export function AppLoader() { return ; } - // Render App with all initialized data + // 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/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; +};