Skip to content

Commit 2379e17

Browse files
committed
🤖 fix: stabilize WorkspaceSidebarState getSnapshot
Change-Id: Ic5fad6fb97672629be20bc11b47ab1809ba01180 Signed-off-by: Thomas Kosiewski <tk@coder.com>
1 parent beaf779 commit 2379e17

File tree

2 files changed

+73
-3
lines changed

2 files changed

+73
-3
lines changed

src/browser/stores/WorkspaceStore.test.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { describe, expect, it, beforeEach, afterEach, mock, type Mock } from "bun:test";
22
import type { FrontendWorkspaceMetadata } from "@/common/types/workspace";
3+
import type { StreamStartEvent, ToolCallStartEvent } from "@/common/types/stream";
34
import type { WorkspaceChatMessage } from "@/common/orpc/types";
45
import { DEFAULT_RUNTIME_CONFIG } from "@/common/constants/workspace";
56
import { WorkspaceStore } from "./WorkspaceStore";
@@ -370,6 +371,61 @@ describe("WorkspaceStore", () => {
370371
expect(state1).toBe(state2);
371372
});
372373

374+
it("getWorkspaceSidebarState() returns same reference when WorkspaceState hasn't changed", () => {
375+
const originalNow = Date.now;
376+
let now = 1000;
377+
Date.now = () => now;
378+
379+
try {
380+
const workspaceId = "test-workspace";
381+
createAndAddWorkspace(store, workspaceId);
382+
383+
const aggregator = store.getAggregator(workspaceId);
384+
expect(aggregator).toBeDefined();
385+
if (!aggregator) {
386+
throw new Error("Expected aggregator to exist");
387+
}
388+
389+
const streamStart: StreamStartEvent = {
390+
type: "stream-start",
391+
workspaceId,
392+
messageId: "msg1",
393+
model: "claude-opus-4",
394+
historySequence: 1,
395+
startTime: 500,
396+
mode: "exec",
397+
};
398+
aggregator.handleStreamStart(streamStart);
399+
400+
const toolStart: ToolCallStartEvent = {
401+
type: "tool-call-start",
402+
workspaceId,
403+
messageId: "msg1",
404+
toolCallId: "tool1",
405+
toolName: "test_tool",
406+
args: {},
407+
tokens: 0,
408+
timestamp: 600,
409+
};
410+
aggregator.handleToolCallStart(toolStart);
411+
412+
// Simulate store update (MapStore version bump) after handling events.
413+
store.bumpState(workspaceId);
414+
415+
now = 1300;
416+
const sidebar1 = store.getWorkspaceSidebarState(workspaceId);
417+
418+
// Advance time without a store bump. Without snapshot caching, this would
419+
// produce a new object due to Date.now()-derived timing stats.
420+
now = 1350;
421+
const sidebar2 = store.getWorkspaceSidebarState(workspaceId);
422+
423+
expect(sidebar2).toBe(sidebar1);
424+
} finally {
425+
Date.now = originalNow;
426+
}
427+
});
428+
373429
it("syncWorkspaces() does not emit when workspaces unchanged", () => {
374430
const listener = mock(() => undefined);
375431
store.subscribe(listener);

src/browser/stores/WorkspaceStore.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -679,6 +679,11 @@ export class WorkspaceStore {
679679

680680
// Cache sidebar state objects to return stable references
681681
private sidebarStateCache = new Map<string, WorkspaceSidebarState>();
682+
// Map from workspaceId -> the WorkspaceState reference used to compute sidebarStateCache.
683+
// React's useSyncExternalStore may call getSnapshot() multiple times per render; this
684+
// ensures getWorkspaceSidebarState() returns a referentially stable snapshot for a given
685+
// MapStore version even when timingStats would otherwise change via Date.now().
686+
private sidebarStateSourceState = new Map<string, WorkspaceState>();
682687

683688
/**
684689
* Get sidebar state for a workspace (subset of full state).
@@ -687,6 +692,12 @@ export class WorkspaceStore {
687692
*/
688693
getWorkspaceSidebarState(workspaceId: string): WorkspaceSidebarState {
689694
const fullState = this.getWorkspaceState(workspaceId);
695+
696+
const cached = this.sidebarStateCache.get(workspaceId);
697+
if (cached && this.sidebarStateSourceState.get(workspaceId) === fullState) {
698+
return cached;
699+
}
700+
690701
const aggregator = this.aggregators.get(workspaceId);
691702

692703
// Get timing stats: prefer active stream, fall back to last completed
@@ -711,9 +722,7 @@ export class WorkspaceStore {
711722
// Get session-level aggregate stats
712723
const sessionStats = aggregator?.getSessionTimingStats() ?? null;
713724

714-
const cached = this.sidebarStateCache.get(workspaceId);
715-
716-
// Return cached if values match (timing stats checked by reference since they change frequently)
725+
// Return cached if values match.
717726
if (
718727
cached &&
719728
cached.canInterrupt === fullState.canInterrupt &&
@@ -728,6 +737,9 @@ export class WorkspaceStore {
728737
(cached.sessionStats?.totalDurationMs === sessionStats?.totalDurationMs &&
729738
cached.sessionStats?.responseCount === sessionStats?.responseCount))
730739
) {
740+
// Even if we re-use the cached object, mark it as derived from the current
741+
// WorkspaceState so repeated getSnapshot() reads during this render are stable.
742+
this.sidebarStateSourceState.set(workspaceId, fullState);
731743
return cached;
732744
}
733745

@@ -742,6 +754,7 @@ export class WorkspaceStore {
742754
sessionStats,
743755
};
744756
this.sidebarStateCache.set(workspaceId, newState);
757+
this.sidebarStateSourceState.set(workspaceId, fullState);
745758
return newState;
746759
}
747760

@@ -1143,6 +1156,7 @@ export class WorkspaceStore {
11431156
this.recencyCache.delete(workspaceId);
11441157
this.previousSidebarValues.delete(workspaceId);
11451158
this.sidebarStateCache.delete(workspaceId);
1159+
this.sidebarStateSourceState.delete(workspaceId);
11461160
this.workspaceCreatedAt.delete(workspaceId);
11471161
this.workspaceStats.delete(workspaceId);
11481162
this.statsStore.delete(workspaceId);

0 commit comments

Comments
 (0)