From 12200ab26a7330049768bb57f7c4e6293d8e5415 Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Sun, 26 Jul 2026 22:13:08 +0200 Subject: [PATCH] fix(workspaces): serialize overlapping topology refreshes The browser-resume path and the plugin-facing app refresh call refreshSelectedProjectTopology independently, so two requests for the same machine and project could be in flight at once. The stale guards check machine and project but not ordering, so a slower earlier response landing last overwrote a newer list, making a just-created worktree disappear again. Route the refresh through TrailingRefreshCoordinator, the primitive already used for browser resume, session refresh, and activity, keyed by machine and project. A second caller no longer opens its own request while one is in flight; it gets a single trailing pass, so the last applied response is the newest one. --- .../controllers/workspaceController.test.ts | 44 +++++++++++++++++++ .../src/controllers/workspaceController.ts | 24 ++++++---- 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/src/client/src/controllers/workspaceController.test.ts b/src/client/src/controllers/workspaceController.test.ts index 4cb98f2..cd189cf 100644 --- a/src/client/src/controllers/workspaceController.test.ts +++ b/src/client/src/controllers/workspaceController.test.ts @@ -273,6 +273,50 @@ describe("WorkspaceController.refreshSelectedProjectTopology", () => { expect(test.state().selectedWorkspace).toBe(selected); }); + it("serializes overlapping refreshes so an earlier response cannot overwrite a newer list", async () => { + const repo = project("p1", "/repo"); + const main = workspace(repo.id, repo.path, { isMain: true }); + const created = workspace(repo.id, "/repo-feature"); + const gates: ((workspaces: Workspace[]) => void)[] = []; + let inFlight = 0; + let maxInFlight = 0; + const loadWorkspaces = vi.fn(() => { + inFlight += 1; + maxInFlight = Math.max(maxInFlight, inFlight); + return new Promise((resolve) => { + gates.push((workspaces) => { inFlight -= 1; resolve(workspaces); }); + }); + }); + const test = harness( + { + selectedMachine: machine("local"), + projects: [repo], + selectedProject: repo, + selectedWorkspace: main, + workspaces: [main], + workspacesByProjectId: { [repo.id]: [main] }, + }, + loadWorkspaces, + ); + + const resumeRefresh = test.controller.refreshSelectedProjectTopology(); + await Promise.resolve(); + const appDataRefresh = test.controller.refreshSelectedProjectTopology(); + await Promise.resolve(); + + // The second caller does not open its own request while the first is in flight; it gets + // one trailing pass afterwards. Without this, two responses race and the slower-but-older + // one can land last, making a just-created worktree disappear again. + expect(maxInFlight).toBe(1); + gates[0]?.([main]); + await vi.waitFor(() => { expect(gates).toHaveLength(2); }); + gates[1]?.([main, created]); + await Promise.all([resumeRefresh, appDataRefresh]); + + // The last response wins, so the newly created worktree stays visible. + expect(test.state().workspaces).toEqual([main, created]); + }); + it("does not request anything when no project is selected", async () => { const loadWorkspaces = vi.fn(); const test = harness({ selectedMachine: machine("local") }, loadWorkspaces); diff --git a/src/client/src/controllers/workspaceController.ts b/src/client/src/controllers/workspaceController.ts index 6a067c8..8158d23 100644 --- a/src/client/src/controllers/workspaceController.ts +++ b/src/client/src/controllers/workspaceController.ts @@ -4,6 +4,7 @@ import { mergeCachedNewSessions } from "../cachedNewSessions"; import { machineProjectKey } from "../machineKeys"; import { selectedMachineId, type GetState, type RouteTarget, type SetState, type UpdateUrl } from "./types"; import type { SessionController } from "./sessionController"; +import { TrailingRefreshCoordinator } from "./trailingRefreshCoordinator"; import { InMemoryWorkspaceSelectionMemory, selectPreferredWorkspace, type WorkspaceSelectionMemory } from "./workspaceSelection"; export interface WorkspaceControllerDependencies { @@ -14,6 +15,7 @@ export interface WorkspaceControllerDependencies { export class WorkspaceController { private readonly api: Pick; private readonly onBackgroundError: (message: string, error: unknown) => void; + private readonly topologyRefreshes = new TrailingRefreshCoordinator(); constructor( private readonly getState: GetState, @@ -99,14 +101,20 @@ export class WorkspaceController { const project = state.selectedProject; if (project === undefined) return; const machineId = selectedMachineId(state); - try { - const workspaces = await this.api.workspaces(project.id, machineId); - const current = this.getState(); - if (selectedMachineId(current) !== machineId || current.selectedProject?.id !== project.id) return; - this.applyProjectWorkspaces(project.id, workspaces); - } catch (error) { - this.onBackgroundError(`Failed to refresh workspaces for project ${project.id} on ${machineId}`, error); - } + // Callers are independent (browser resume and the plugin-facing app refresh), so two + // refreshes for the same machine+project can overlap. Sharing one request keeps a slow + // earlier response from landing last and overwriting a newer list, which would make a + // just-created worktree disappear again. + await this.topologyRefreshes.request(machineProjectKey(machineId, project.id), async () => { + try { + const workspaces = await this.api.workspaces(project.id, machineId); + const current = this.getState(); + if (selectedMachineId(current) !== machineId || current.selectedProject?.id !== project.id) return; + this.applyProjectWorkspaces(project.id, workspaces); + } catch (error) { + this.onBackgroundError(`Failed to refresh workspaces for project ${project.id} on ${machineId}`, error); + } + }); } async refreshAfterWorkspaceDeleted(projectId: string, workspaceId: string): Promise {