From 92c39c8f3594c871b71341f333edb52916e6c0dd Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Sun, 26 Jul 2026 22:04:54 +0200 Subject: [PATCH] fix(workspaces): keep selected workspace metadata fresh on refresh A background topology refresh replaced the workspace list but left selectedWorkspace pointing at the old object, so a branch switched inside a worktree outside PI WEB showed the new name in the list while the collapsed Workspaces header and the mobile context bar kept the old one until reselect. Re-point selectedWorkspace at its refreshed entry, keyed by id (derived from the path), so which workspace is selected never changes and the session and terminal teardown in handleWorkspaceChange still does not fire. Skip the patch entirely when metadata is unchanged, so an ordinary resume does not churn object identity into state on every focus. --- .../controllers/workspaceController.test.ts | 53 +++++++++++++++++++ .../src/controllers/workspaceController.ts | 33 ++++++++++-- 2 files changed, 83 insertions(+), 3 deletions(-) diff --git a/src/client/src/controllers/workspaceController.test.ts b/src/client/src/controllers/workspaceController.test.ts index ea92d88..4cb98f2 100644 --- a/src/client/src/controllers/workspaceController.test.ts +++ b/src/client/src/controllers/workspaceController.test.ts @@ -220,6 +220,59 @@ describe("WorkspaceController.refreshSelectedProjectTopology", () => { expect(test.backgroundErrors).toEqual([{ message: `Failed to refresh workspaces for project ${repo.id} on local`, error: failure }]); }); + it("re-points the selected workspace at refreshed metadata when its branch changed outside PI WEB", async () => { + const repo = project("p1", "/repo"); + const main = workspace(repo.id, repo.path, { isMain: true }); + const selected = { ...workspace(repo.id, "/repo-feature"), label: "feature-a", branch: "feature-a" }; + const switched = { ...selected, label: "feature-b", branch: "feature-b" }; + const loadWorkspaces = vi.fn().mockResolvedValue([main, switched]); + const test = harness( + { + selectedMachine: machine("local"), + projects: [repo], + selectedProject: repo, + selectedWorkspace: selected, + workspaces: [main, selected], + workspacesByProjectId: { [repo.id]: [main, selected] }, + selectedSession: session(selected.path), + }, + loadWorkspaces, + ); + + await test.controller.refreshSelectedProjectTopology(); + + // Same workspace (same id/path), so the session must survive; only the stale label moves. + expect(test.state().selectedWorkspace).toEqual(switched); + expect(test.state().selectedWorkspace?.id).toBe(selected.id); + expect(test.state().selectedSession).toBeDefined(); + expect(test.clearActiveSession).not.toHaveBeenCalled(); + }); + + it("leaves the selected workspace object untouched when the refresh returns identical metadata", async () => { + const repo = project("p1", "/repo"); + const main = workspace(repo.id, repo.path, { isMain: true }); + const selected = workspace(repo.id, "/repo-feature"); + // A fresh, equal object, exactly what a real HTTP response produces every resume. + const loadWorkspaces = vi.fn().mockResolvedValue([{ ...main }, { ...selected }]); + const test = harness( + { + selectedMachine: machine("local"), + projects: [repo], + selectedProject: repo, + selectedWorkspace: selected, + workspaces: [main, selected], + workspacesByProjectId: { [repo.id]: [main, selected] }, + }, + loadWorkspaces, + ); + + await test.controller.refreshSelectedProjectTopology(); + + // Identity preserved: an unchanged resume must not churn selected-workspace identity + // into state, or every focus would re-render surfaces keyed on this object. + expect(test.state().selectedWorkspace).toBe(selected); + }); + 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 581da29..6a067c8 100644 --- a/src/client/src/controllers/workspaceController.ts +++ b/src/client/src/controllers/workspaceController.ts @@ -1,5 +1,5 @@ import { api as defaultApi, type Project, type Workspace } from "../api"; -import { resetWorkspaceScopedState } from "../appState"; +import { resetWorkspaceScopedState, type AppState } from "../appState"; import { mergeCachedNewSessions } from "../cachedNewSessions"; import { machineProjectKey } from "../machineKeys"; import { selectedMachineId, type GetState, type RouteTarget, type SetState, type UpdateUrl } from "./types"; @@ -122,8 +122,26 @@ export class WorkspaceController { private applyProjectWorkspaces(projectId: string, workspaces: Workspace[]): void { const state = this.getState(); const workspacesByProjectId = { ...state.workspacesByProjectId, [projectId]: workspaces }; - if (state.selectedProject?.id === projectId) this.setState({ workspaces, workspacesByProjectId }); - else this.setState({ workspacesByProjectId }); + if (state.selectedProject?.id !== projectId) { + this.setState({ workspacesByProjectId }); + return; + } + this.setState({ workspaces, workspacesByProjectId, ...this.refreshedSelection(state.selectedWorkspace, workspaces) }); + } + + /** + * Re-points `selectedWorkspace` at its refreshed entry when metadata changed outside PI WEB + * (a branch switched in the worktree, say), so the workspace list and the surfaces that read + * the selected workspace cannot disagree. Keyed by id, which is derived from the path, so + * this never changes *which* workspace is selected and never triggers the session/terminal + * teardown in `handleWorkspaceChange`. Returns nothing when the entry is gone or unchanged, + * so an unchanged refresh does not churn object identity into state. + */ + private refreshedSelection(selected: Workspace | undefined, workspaces: Workspace[]): Pick | undefined { + if (selected === undefined) return undefined; + const refreshed = workspaces.find((candidate) => candidate.id === selected.id); + if (refreshed === undefined || sameWorkspaceMetadata(selected, refreshed)) return undefined; + return { selectedWorkspace: refreshed }; } } @@ -135,3 +153,12 @@ function selectFallbackWorkspace(workspaces: Workspace[]): Workspace | undefined return workspaces.find((workspace) => workspace.isMain) ?? workspaces[0]; } +function sameWorkspaceMetadata(left: Workspace, right: Workspace): boolean { + return left.path === right.path + && left.label === right.label + && left.branch === right.branch + && left.isMain === right.isMain + && left.isGitRepo === right.isGitRepo + && left.isGitWorktree === right.isGitWorktree; +} +