From 2379e1755f70b4a03c7060ae9323e016a6a0d6a7 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 19 Dec 2025 18:40:54 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=A4=96=20fix:=20stabilize=20WorkspaceSide?= =?UTF-8?q?barState=20getSnapshot?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: Ic5fad6fb97672629be20bc11b47ab1809ba01180 Signed-off-by: Thomas Kosiewski --- src/browser/stores/WorkspaceStore.test.ts | 56 +++++++++++++++++++++++ src/browser/stores/WorkspaceStore.ts | 20 ++++++-- 2 files changed, 73 insertions(+), 3 deletions(-) diff --git a/src/browser/stores/WorkspaceStore.test.ts b/src/browser/stores/WorkspaceStore.test.ts index 691d2f955f..767e8fd1a8 100644 --- a/src/browser/stores/WorkspaceStore.test.ts +++ b/src/browser/stores/WorkspaceStore.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, beforeEach, afterEach, mock, type Mock } from "bun:test"; import type { FrontendWorkspaceMetadata } from "@/common/types/workspace"; +import type { StreamStartEvent, ToolCallStartEvent } from "@/common/types/stream"; import type { WorkspaceChatMessage } from "@/common/orpc/types"; import { DEFAULT_RUNTIME_CONFIG } from "@/common/constants/workspace"; import { WorkspaceStore } from "./WorkspaceStore"; @@ -370,6 +371,61 @@ describe("WorkspaceStore", () => { expect(state1).toBe(state2); }); + it("getWorkspaceSidebarState() returns same reference when WorkspaceState hasn't changed", () => { + const originalNow = Date.now; + let now = 1000; + Date.now = () => now; + + try { + const workspaceId = "test-workspace"; + createAndAddWorkspace(store, workspaceId); + + const aggregator = store.getAggregator(workspaceId); + expect(aggregator).toBeDefined(); + if (!aggregator) { + throw new Error("Expected aggregator to exist"); + } + + const streamStart: StreamStartEvent = { + type: "stream-start", + workspaceId, + messageId: "msg1", + model: "claude-opus-4", + historySequence: 1, + startTime: 500, + mode: "exec", + }; + aggregator.handleStreamStart(streamStart); + + const toolStart: ToolCallStartEvent = { + type: "tool-call-start", + workspaceId, + messageId: "msg1", + toolCallId: "tool1", + toolName: "test_tool", + args: {}, + tokens: 0, + timestamp: 600, + }; + aggregator.handleToolCallStart(toolStart); + + // Simulate store update (MapStore version bump) after handling events. + store.bumpState(workspaceId); + + now = 1300; + const sidebar1 = store.getWorkspaceSidebarState(workspaceId); + + // Advance time without a store bump. Without snapshot caching, this would + // produce a new object due to Date.now()-derived timing stats. + now = 1350; + const sidebar2 = store.getWorkspaceSidebarState(workspaceId); + + expect(sidebar2).toBe(sidebar1); + } finally { + Date.now = originalNow; + } + }); + it("syncWorkspaces() does not emit when workspaces unchanged", () => { const listener = mock(() => undefined); store.subscribe(listener); diff --git a/src/browser/stores/WorkspaceStore.ts b/src/browser/stores/WorkspaceStore.ts index 2b1a5706dd..009b989415 100644 --- a/src/browser/stores/WorkspaceStore.ts +++ b/src/browser/stores/WorkspaceStore.ts @@ -679,6 +679,11 @@ export class WorkspaceStore { // Cache sidebar state objects to return stable references private sidebarStateCache = new Map(); + // Map from workspaceId -> the WorkspaceState reference used to compute sidebarStateCache. + // React's useSyncExternalStore may call getSnapshot() multiple times per render; this + // ensures getWorkspaceSidebarState() returns a referentially stable snapshot for a given + // MapStore version even when timingStats would otherwise change via Date.now(). + private sidebarStateSourceState = new Map(); /** * Get sidebar state for a workspace (subset of full state). @@ -687,6 +692,12 @@ export class WorkspaceStore { */ getWorkspaceSidebarState(workspaceId: string): WorkspaceSidebarState { const fullState = this.getWorkspaceState(workspaceId); + + const cached = this.sidebarStateCache.get(workspaceId); + if (cached && this.sidebarStateSourceState.get(workspaceId) === fullState) { + return cached; + } + const aggregator = this.aggregators.get(workspaceId); // Get timing stats: prefer active stream, fall back to last completed @@ -711,9 +722,7 @@ export class WorkspaceStore { // Get session-level aggregate stats const sessionStats = aggregator?.getSessionTimingStats() ?? null; - const cached = this.sidebarStateCache.get(workspaceId); - - // Return cached if values match (timing stats checked by reference since they change frequently) + // Return cached if values match. if ( cached && cached.canInterrupt === fullState.canInterrupt && @@ -728,6 +737,9 @@ export class WorkspaceStore { (cached.sessionStats?.totalDurationMs === sessionStats?.totalDurationMs && cached.sessionStats?.responseCount === sessionStats?.responseCount)) ) { + // Even if we re-use the cached object, mark it as derived from the current + // WorkspaceState so repeated getSnapshot() reads during this render are stable. + this.sidebarStateSourceState.set(workspaceId, fullState); return cached; } @@ -742,6 +754,7 @@ export class WorkspaceStore { sessionStats, }; this.sidebarStateCache.set(workspaceId, newState); + this.sidebarStateSourceState.set(workspaceId, fullState); return newState; } @@ -1143,6 +1156,7 @@ export class WorkspaceStore { this.recencyCache.delete(workspaceId); this.previousSidebarValues.delete(workspaceId); this.sidebarStateCache.delete(workspaceId); + this.sidebarStateSourceState.delete(workspaceId); this.workspaceCreatedAt.delete(workspaceId); this.workspaceStats.delete(workspaceId); this.statsStore.delete(workspaceId);