From efd4a5550425dd4a2c78f8c6e5f3c52938398cc2 Mon Sep 17 00:00:00 2001 From: Mike Olson Date: Sun, 19 Apr 2026 22:04:30 -0400 Subject: [PATCH 1/3] fix(desktop): reveal window on did-finish-load fallback to avoid Wayland deadlock On Linux/Wayland, Electron's `ready-to-show` event only fires after `show()` is called when the BrowserWindow is created with `show: false`, because the wl_surface has no role assigned until then and the compositor never reports the surface as ready. The standard "wait for ready, then show" pattern deadlocks: nothing ever calls `show()`, so the window never appears. Add `did-finish-load` as a Linux-only fallback trigger so the first event from either source reveals the window. Other platforms keep the no-flash `ready-to-show` path, since `did-finish-load` typically fires before the first paint there. Extract the bind-once logic into `bindFirstRevealTrigger` with focused unit tests. --- apps/desktop/src/main.ts | 22 ++++---- apps/desktop/src/windowReveal.test.ts | 74 +++++++++++++++++++++++++++ apps/desktop/src/windowReveal.ts | 28 ++++++++++ 3 files changed, 114 insertions(+), 10 deletions(-) create mode 100644 apps/desktop/src/windowReveal.test.ts create mode 100644 apps/desktop/src/windowReveal.ts diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index 529ed55d03f..c5507c6fb03 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -75,6 +75,7 @@ import { } from "./updateMachine.ts"; import { isArm64HostRunningIntelBuild, resolveDesktopRuntimeInfo } from "./runtimeArch.ts"; import { resolveDesktopAppBranding } from "./appBranding.ts"; +import { bindFirstRevealTrigger, type RevealSubscription } from "./windowReveal.ts"; syncShellEnvironment(); @@ -1998,16 +1999,17 @@ function createWindow(): BrowserWindow { emitUpdateState(); }); - let initialRevealScheduled = false; - const revealInitialWindow = () => { - if (initialRevealScheduled) { - return; - } - initialRevealScheduled = true; - revealWindow(window); - }; - - window.once("ready-to-show", revealInitialWindow); + // On Linux/Wayland with `show: false`, Electron's `ready-to-show` only + // fires after `show()` is called, deadlocking the standard "wait for + // ready, then show" pattern. Add `did-finish-load` as a Linux-only + // fallback so the window still surfaces once the renderer has loaded + // the page. Other platforms keep the no-flash `ready-to-show` path, + // since `did-finish-load` typically fires before the first paint there. + const revealSubscribers: RevealSubscription[] = [(fire) => window.once("ready-to-show", fire)]; + if (process.platform === "linux") { + revealSubscribers.push((fire) => window.webContents.once("did-finish-load", fire)); + } + bindFirstRevealTrigger(revealSubscribers, () => revealWindow(window)); if (isDevelopment) { void window.loadURL(resolveDesktopDevServerUrl()); diff --git a/apps/desktop/src/windowReveal.test.ts b/apps/desktop/src/windowReveal.test.ts new file mode 100644 index 00000000000..88285fec0cb --- /dev/null +++ b/apps/desktop/src/windowReveal.test.ts @@ -0,0 +1,74 @@ +import { EventEmitter } from "node:events"; + +import { describe, expect, it, vi } from "vitest"; + +import { bindFirstRevealTrigger } from "./windowReveal.ts"; + +describe("bindFirstRevealTrigger", () => { + it("reveals when the first trigger fires", () => { + const window = new EventEmitter(); + const webContents = new EventEmitter(); + const reveal = vi.fn(); + + bindFirstRevealTrigger( + [ + (fire) => window.once("ready-to-show", fire), + (fire) => webContents.once("did-finish-load", fire), + ], + reveal, + ); + + window.emit("ready-to-show"); + + expect(reveal).toHaveBeenCalledTimes(1); + }); + + it("reveals when only the fallback trigger fires (Wayland deadlock case)", () => { + const window = new EventEmitter(); + const webContents = new EventEmitter(); + const reveal = vi.fn(); + + bindFirstRevealTrigger( + [ + (fire) => window.once("ready-to-show", fire), + (fire) => webContents.once("did-finish-load", fire), + ], + reveal, + ); + + webContents.emit("did-finish-load"); + + expect(reveal).toHaveBeenCalledTimes(1); + }); + + it("only reveals once when multiple triggers fire", () => { + const window = new EventEmitter(); + const webContents = new EventEmitter(); + const reveal = vi.fn(); + + bindFirstRevealTrigger( + [ + (fire) => window.once("ready-to-show", fire), + (fire) => webContents.once("did-finish-load", fire), + ], + reveal, + ); + + webContents.emit("did-finish-load"); + window.emit("ready-to-show"); + + expect(reveal).toHaveBeenCalledTimes(1); + }); + + it("subscribers using `once` ignore re-emitted events after reveal", () => { + const window = new EventEmitter(); + const reveal = vi.fn(); + + bindFirstRevealTrigger([(fire) => window.once("ready-to-show", fire)], reveal); + + window.emit("ready-to-show"); + window.emit("ready-to-show"); + + expect(reveal).toHaveBeenCalledTimes(1); + }); +}); diff --git a/apps/desktop/src/windowReveal.ts b/apps/desktop/src/windowReveal.ts new file mode 100644 index 00000000000..8faf65aeb15 --- /dev/null +++ b/apps/desktop/src/windowReveal.ts @@ -0,0 +1,28 @@ +export type RevealSubscription = (listener: () => void) => void; + +/** + * Wire a reveal callback to fire exactly once, on whichever of the provided + * event subscribers fires first. Each subscriber is responsible for binding + * its own event source. + * + * Used by the desktop main window's first-paint reveal logic. The standard + * Electron pattern is to wait for `ready-to-show` before calling `show()`, + * but on Linux/Wayland with `show: false`, `ready-to-show` only fires after + * `show()` is called, deadlocking that pattern. Subscribing to both + * `ready-to-show` and `did-finish-load` (or any other "renderer is alive" + * signal) lets the window surface reliably across platforms. + */ +export function bindFirstRevealTrigger( + subscribers: readonly RevealSubscription[], + reveal: () => void, +): void { + let revealed = false; + const fire = () => { + if (revealed) return; + revealed = true; + reveal(); + }; + for (const subscribe of subscribers) { + subscribe(fire); + } +} From c2d873f222d9c6af3194b46a688c480f32f60aab Mon Sep 17 00:00:00 2001 From: Mike Olson Date: Sun, 19 Apr 2026 23:24:03 -0400 Subject: [PATCH 2/3] fix(web): restore manual sort drag and keep per-group expand state #2055 silently regressed two sidebar behaviors by changing how projects are keyed in `uiStateStore`: - Manual-sort drag reorder snapped projects back to their original position. Writers populated `projectOrder` with physical keys (env + cwd) but readers matched items by scoped keys (env + project id), so `preferredIds` never matched and manual order was discarded. - Expand/collapse state was wiped whenever projects were grouped (grouping != `separate`), because `syncProjects` seeded `projectExpandedById` by physical key while the UI read and wrote it by logical (group) key. Route readers and writers through a single source of truth: - `getProjectOrderKey` centralizes the physical-key derivation used by `projectOrder` so future callers cannot silently drift again. - `SyncProjectInput` now carries both a physical `key` (sort order) and a `logicalKey` (expand/collapse). `syncProjects` keys `projectExpandedById` by `logicalKey`, and persistence tracks the member cwds per group so localStorage hydration still works for grouped projects. --- apps/web/src/components/Sidebar.logic.test.ts | 36 ++++++++++ apps/web/src/components/Sidebar.tsx | 8 ++- apps/web/src/environments/runtime/service.ts | 10 ++- apps/web/src/hooks/useHandleNewThread.ts | 4 +- apps/web/src/hooks/useSettings.ts | 9 +++ apps/web/src/logicalProject.ts | 7 ++ apps/web/src/uiStateStore.test.ts | 68 ++++++++++++++----- apps/web/src/uiStateStore.ts | 43 +++++++----- 8 files changed, 146 insertions(+), 39 deletions(-) diff --git a/apps/web/src/components/Sidebar.logic.test.ts b/apps/web/src/components/Sidebar.logic.test.ts index 6aa042a169e..f92f2f628c3 100644 --- a/apps/web/src/components/Sidebar.logic.test.ts +++ b/apps/web/src/components/Sidebar.logic.test.ts @@ -309,6 +309,42 @@ describe("orderItemsByPreferredIds", () => { ProjectId.make("project-1"), ]); }); + + it("honors projectOrder physical keys via getProjectOrderKey", async () => { + // Regression guard for #1904 / the regression introduced by #2055: + // `projectOrder` is populated with physical keys (envId + cwd-derived) + // by the store and by drag-end handlers. Readers must identify projects + // with the same key format, or manual sort silently snaps back. + const { getProjectOrderKey } = await import("../logicalProject"); + const projects = [ + { + environmentId: EnvironmentId.make("environment-local"), + id: ProjectId.make("id-alpha"), + cwd: "/work/alpha", + }, + { + environmentId: EnvironmentId.make("environment-local"), + id: ProjectId.make("id-beta"), + cwd: "/work/beta", + }, + { + environmentId: EnvironmentId.make("environment-local"), + id: ProjectId.make("id-gamma"), + cwd: "/work/gamma", + }, + ]; + const ordered = orderItemsByPreferredIds({ + items: projects, + preferredIds: [getProjectOrderKey(projects[2]!), getProjectOrderKey(projects[0]!)], + getId: getProjectOrderKey, + }); + + expect(ordered.map((project) => project.cwd)).toEqual([ + "/work/gamma", + "/work/alpha", + "/work/beta", + ]); + }); }); describe("resolveAdjacentThreadId", () => { diff --git a/apps/web/src/components/Sidebar.tsx b/apps/web/src/components/Sidebar.tsx index 8335042c110..d25f930c0d5 100644 --- a/apps/web/src/components/Sidebar.tsx +++ b/apps/web/src/components/Sidebar.tsx @@ -168,7 +168,11 @@ import { CommandDialogTrigger } from "./ui/command"; import { readEnvironmentApi } from "../environmentApi"; import { useSettings, useUpdateSettings } from "~/hooks/useSettings"; import { useServerKeybindings } from "../rpc/serverState"; -import { derivePhysicalProjectKey, deriveProjectGroupingOverrideKey } from "../logicalProject"; +import { + derivePhysicalProjectKey, + deriveProjectGroupingOverrideKey, + getProjectOrderKey, +} from "../logicalProject"; import { useSavedEnvironmentRegistryStore, useSavedEnvironmentRuntimeStore, @@ -2708,7 +2712,7 @@ export default function Sidebar() { return orderItemsByPreferredIds({ items: projects, preferredIds: projectOrder, - getId: (project) => scopedProjectKey(scopeProjectRef(project.environmentId, project.id)), + getId: getProjectOrderKey, }); }, [projectOrder, projects]); diff --git a/apps/web/src/environments/runtime/service.ts b/apps/web/src/environments/runtime/service.ts index 40f8f8f2c00..775df0184eb 100644 --- a/apps/web/src/environments/runtime/service.ts +++ b/apps/web/src/environments/runtime/service.ts @@ -61,7 +61,11 @@ import { useTerminalStateStore } from "~/terminalStateStore"; import { useUiStateStore } from "~/uiStateStore"; import { WsTransport } from "../../rpc/wsTransport"; import { createWsRpcClient, type WsRpcClient } from "../../rpc/wsRpcClient"; -import { derivePhysicalProjectKey } from "../../logicalProject"; +import { + deriveLogicalProjectKeyFromSettings, + derivePhysicalProjectKey, +} from "../../logicalProject"; +import { getClientSettings } from "~/hooks/useSettings"; type EnvironmentServiceState = { readonly queryClient: QueryClient; @@ -468,9 +472,11 @@ function coalesceOrchestrationUiEvents( function syncProjectUiFromStore() { const projects = selectProjectsAcrossEnvironments(useStore.getState()); + const clientSettings = getClientSettings(); useUiStateStore.getState().syncProjects( projects.map((project) => ({ key: derivePhysicalProjectKey(project), + logicalKey: deriveLogicalProjectKeyFromSettings(project, clientSettings), cwd: project.cwd, })), ); @@ -541,9 +547,11 @@ function applyRecoveredEventBatch( useStore.getState().applyOrchestrationEvents(uiEvents, environmentId); if (needsProjectUiSync) { const projects = selectProjectsAcrossEnvironments(useStore.getState()); + const clientSettings = getClientSettings(); useUiStateStore.getState().syncProjects( projects.map((project) => ({ key: derivePhysicalProjectKey(project), + logicalKey: deriveLogicalProjectKeyFromSettings(project, clientSettings), cwd: project.cwd, })), ); diff --git a/apps/web/src/hooks/useHandleNewThread.ts b/apps/web/src/hooks/useHandleNewThread.ts index d512b6c7e76..3630171bf59 100644 --- a/apps/web/src/hooks/useHandleNewThread.ts +++ b/apps/web/src/hooks/useHandleNewThread.ts @@ -10,7 +10,7 @@ import { } from "../composerDraftStore"; import { newDraftId, newThreadId } from "../lib/utils"; import { orderItemsByPreferredIds } from "../components/Sidebar.logic"; -import { deriveLogicalProjectKeyFromSettings } from "../logicalProject"; +import { deriveLogicalProjectKeyFromSettings, getProjectOrderKey } from "../logicalProject"; import { selectProjectsAcrossEnvironments, useStore } from "../store"; import { createThreadSelectorByRef } from "../storeSelectors"; import { resolveThreadRouteTarget } from "../threadRoutes"; @@ -169,7 +169,7 @@ export function useHandleNewThread() { return orderItemsByPreferredIds({ items: projects, preferredIds: projectOrder, - getId: (project) => scopedProjectKey(scopeProjectRef(project.environmentId, project.id)), + getId: getProjectOrderKey, }); }, [projectOrder, projects]); const handleNewThread = useNewThreadState(); diff --git a/apps/web/src/hooks/useSettings.ts b/apps/web/src/hooks/useSettings.ts index 18b2668a603..37a6872bd92 100644 --- a/apps/web/src/hooks/useSettings.ts +++ b/apps/web/src/hooks/useSettings.ts @@ -123,6 +123,15 @@ function splitPatch(patch: Partial): { * only re-render when the slice they care about changes. */ +/** + * Non-hook accessor for the current merged client settings snapshot. + * Used by non-React code paths (e.g. runtime services) that need the latest + * settings without subscribing. + */ +export function getClientSettings(): ClientSettings { + return getClientSettingsSnapshot(); +} + export function useSettings(selector?: (s: UnifiedSettings) => T): T { const serverSettings = useServerSettings(); const clientSettings = useSyncExternalStore( diff --git a/apps/web/src/logicalProject.ts b/apps/web/src/logicalProject.ts index d30bb60ca06..f19b64ac80d 100644 --- a/apps/web/src/logicalProject.ts +++ b/apps/web/src/logicalProject.ts @@ -65,6 +65,13 @@ export function deriveProjectGroupingOverrideKey( return derivePhysicalProjectKey(project); } +// Key under which a project's manual sort order (projectOrder) is stored. +// Must stay aligned with the writer side in `uiStateStore.syncProjects` and +// the drag handlers in `Sidebar` so readers and writers agree. +export function getProjectOrderKey(project: Pick): string { + return derivePhysicalProjectKey(project); +} + export function resolveProjectGroupingMode( project: Pick, settings: ProjectGroupingSettings, diff --git a/apps/web/src/uiStateStore.test.ts b/apps/web/src/uiStateStore.test.ts index c906bbc1d78..faf7e62774d 100644 --- a/apps/web/src/uiStateStore.test.ts +++ b/apps/web/src/uiStateStore.test.ts @@ -183,40 +183,43 @@ describe("uiStateStore pure functions", () => { }); const next = syncProjects(initialState, [ - { key: project1, cwd: "/tmp/project-1" }, - { key: project2, cwd: "/tmp/project-2" }, - { key: project3, cwd: "/tmp/project-3" }, + { key: project1, logicalKey: project1, cwd: "/tmp/project-1" }, + { key: project2, logicalKey: project2, cwd: "/tmp/project-2" }, + { key: project3, logicalKey: project3, cwd: "/tmp/project-3" }, ]); expect(next.projectOrder).toEqual([project2, project1, project3]); expect(next.projectExpandedById[project2]).toBe(false); }); - it("syncProjects preserves manual order when a project is recreated with the same cwd", () => { - const oldProject1 = ProjectId.make("project-1"); - const oldProject2 = ProjectId.make("project-2"); - const recreatedProject2 = ProjectId.make("project-2b"); + it("syncProjects preserves manual order across project id churn at the same cwd", () => { + // Under the current design, physical key and logical key are both + // cwd-derived, so an internal project-id change doesn't alter the store + // keys. This test locks in that stability: re-syncing the same cwds keeps + // manual order and collapse state. + const keyProject1 = "env-local:/tmp/project-1"; + const keyProject2 = "env-local:/tmp/project-2"; const initialState = syncProjects( makeUiState({ projectExpandedById: { - [oldProject1]: true, - [oldProject2]: false, + [keyProject1]: true, + [keyProject2]: false, }, - projectOrder: [oldProject2, oldProject1], + projectOrder: [keyProject2, keyProject1], }), [ - { key: oldProject1, cwd: "/tmp/project-1" }, - { key: oldProject2, cwd: "/tmp/project-2" }, + { key: keyProject1, logicalKey: keyProject1, cwd: "/tmp/project-1" }, + { key: keyProject2, logicalKey: keyProject2, cwd: "/tmp/project-2" }, ], ); const next = syncProjects(initialState, [ - { key: oldProject1, cwd: "/tmp/project-1" }, - { key: recreatedProject2, cwd: "/tmp/project-2" }, + { key: keyProject1, logicalKey: keyProject1, cwd: "/tmp/project-1" }, + { key: keyProject2, logicalKey: keyProject2, cwd: "/tmp/project-2" }, ]); - expect(next.projectOrder).toEqual([recreatedProject2, oldProject1]); - expect(next.projectExpandedById[recreatedProject2]).toBe(false); + expect(next.projectOrder).toEqual([keyProject2, keyProject1]); + expect(next.projectExpandedById[keyProject2]).toBe(false); }); it("syncProjects returns a new state when only project cwd changes", () => { @@ -228,16 +231,45 @@ describe("uiStateStore pure functions", () => { }, projectOrder: [project1], }), - [{ key: project1, cwd: "/tmp/project-1" }], + [{ key: project1, logicalKey: project1, cwd: "/tmp/project-1" }], ); - const next = syncProjects(initialState, [{ key: project1, cwd: "/tmp/project-1-renamed" }]); + const next = syncProjects(initialState, [ + { key: project1, logicalKey: project1, cwd: "/tmp/project-1-renamed" }, + ]); expect(next).not.toBe(initialState); expect(next.projectOrder).toEqual([project1]); expect(next.projectExpandedById[project1]).toBe(false); }); + it("syncProjects keys projectExpandedById by the logical key, not the physical key", () => { + // In repository grouping mode, multiple physical projects (different + // environments or different repo-relative paths) collapse into one + // logical group. The group's expand state must be keyed by the logical + // key so clicks on the grouped row toggle the shared state, and so the + // state survives subsequent syncProjects calls (which rebuild the map + // from incoming inputs). + const physicalLocal = "env-local:/repo/project"; + const physicalRemote = "env-remote:/repo/project"; + const logicalKey = "repo-canonical-key"; + + const initial = syncProjects(makeUiState(), [ + { key: physicalLocal, logicalKey, cwd: "/repo/project" }, + { key: physicalRemote, logicalKey, cwd: "/repo/project" }, + ]); + + expect(initial.projectExpandedById).toEqual({ [logicalKey]: true }); + + const afterCollapse = { ...initial, projectExpandedById: { [logicalKey]: false } }; + const next = syncProjects(afterCollapse, [ + { key: physicalLocal, logicalKey, cwd: "/repo/project" }, + { key: physicalRemote, logicalKey, cwd: "/repo/project" }, + ]); + + expect(next.projectExpandedById[logicalKey]).toBe(false); + }); + it("syncThreads prunes missing thread UI state", () => { const thread1 = ThreadId.make("thread-1"); const thread2 = ThreadId.make("thread-2"); diff --git a/apps/web/src/uiStateStore.ts b/apps/web/src/uiStateStore.ts index 7ae72320630..eab7a68fe1b 100644 --- a/apps/web/src/uiStateStore.ts +++ b/apps/web/src/uiStateStore.ts @@ -34,7 +34,10 @@ export interface UiThreadState { export interface UiState extends UiProjectState, UiThreadState {} export interface SyncProjectInput { + /** Physical project key (env + cwd). Used for manual sort order. */ key: string; + /** Logical group key. Used for expand/collapse state. */ + logicalKey: string; cwd: string; } @@ -53,6 +56,7 @@ const initialState: UiState = { const persistedExpandedProjectCwds = new Set(); const persistedProjectOrderCwds: string[] = []; const currentProjectCwdById = new Map(); +const currentProjectCwdsByLogicalKey = new Map(); let legacyKeysCleanedUp = false; function readPersistedState(): UiState { @@ -135,10 +139,7 @@ function persistState(state: UiState): void { try { const expandedProjectCwds = Object.entries(state.projectExpandedById) .filter(([, expanded]) => expanded) - .flatMap(([projectId]) => { - const cwd = currentProjectCwdById.get(projectId); - return cwd ? [cwd] : []; - }); + .flatMap(([logicalKey]) => currentProjectCwdsByLogicalKey.get(logicalKey) ?? []); const projectOrderCwds = state.projectOrder.flatMap((projectId) => { const cwd = currentProjectCwdById.get(projectId); return cwd ? [cwd] : []; @@ -211,13 +212,21 @@ function nestedBooleanRecordsEqual( export function syncProjects(state: UiState, projects: readonly SyncProjectInput[]): UiState { const previousProjectCwdById = new Map(currentProjectCwdById); - const previousProjectIdByCwd = new Map( - [...previousProjectCwdById.entries()].map(([projectId, cwd]) => [cwd, projectId] as const), - ); currentProjectCwdById.clear(); for (const project of projects) { currentProjectCwdById.set(project.key, project.cwd); } + currentProjectCwdsByLogicalKey.clear(); + for (const project of projects) { + const cwds = currentProjectCwdsByLogicalKey.get(project.logicalKey); + if (cwds) { + if (!cwds.includes(project.cwd)) { + cwds.push(project.cwd); + } + } else { + currentProjectCwdsByLogicalKey.set(project.logicalKey, [project.cwd]); + } + } const cwdMappingChanged = previousProjectCwdById.size !== currentProjectCwdById.size || projects.some((project) => previousProjectCwdById.get(project.key) !== project.cwd); @@ -228,14 +237,15 @@ export function syncProjects(state: UiState, projects: readonly SyncProjectInput persistedProjectOrderCwds.map((cwd, index) => [cwd, index] as const), ); const mappedProjects = projects.map((project, index) => { - const previousProjectIdForCwd = previousProjectIdByCwd.get(project.cwd); - const expanded = - previousExpandedById[project.key] ?? - (previousProjectIdForCwd ? previousExpandedById[previousProjectIdForCwd] : undefined) ?? - (persistedExpandedProjectCwds.size > 0 - ? persistedExpandedProjectCwds.has(project.cwd) - : true); - nextExpandedById[project.key] = expanded; + if (!(project.logicalKey in nextExpandedById)) { + const groupCwds = currentProjectCwdsByLogicalKey.get(project.logicalKey) ?? [project.cwd]; + const expanded = + previousExpandedById[project.logicalKey] ?? + (persistedExpandedProjectCwds.size > 0 + ? groupCwds.some((cwd) => persistedExpandedProjectCwds.has(cwd)) + : true); + nextExpandedById[project.logicalKey] = expanded; + } return { id: project.key, cwd: project.cwd, @@ -246,6 +256,7 @@ export function syncProjects(state: UiState, projects: readonly SyncProjectInput const nextProjectOrder = state.projectOrder.length > 0 ? (() => { + const currentProjectIds = new Set(mappedProjects.map((project) => project.id)); const nextProjectIdByCwd = new Map( mappedProjects.map((project) => [project.cwd, project.id] as const), ); @@ -254,7 +265,7 @@ export function syncProjects(state: UiState, projects: readonly SyncProjectInput for (const projectId of state.projectOrder) { const matchedProjectId = - (projectId in nextExpandedById ? projectId : undefined) ?? + (currentProjectIds.has(projectId) ? projectId : undefined) ?? (() => { const previousCwd = previousProjectCwdById.get(projectId); return previousCwd ? nextProjectIdByCwd.get(previousCwd) : undefined; From 3814b37578fabc5e98fcf7d7fd2deda66b9e27f8 Mon Sep 17 00:00:00 2001 From: Mike Olson Date: Sun, 19 Apr 2026 23:32:06 -0400 Subject: [PATCH 3/3] fix(web): preserve project expand state when logical key changes When late-arriving project metadata flips the grouping identity for a project (e.g. physical key to repository canonical key), the row itself did not change but its entry in `projectExpandedById` moved to a new key. Before this change, we fell through to the persisted-cwds fallback, which often meant silently defaulting the row back to expanded. Track the previous logical key per physical key across syncs, then if a project's new logical key is unseen, fall back to the previous logical key's expand state before touching the persisted fallback. --- apps/web/src/uiStateStore.test.ts | 25 +++++++++++++++++++++++ apps/web/src/uiStateStore.ts | 34 +++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/apps/web/src/uiStateStore.test.ts b/apps/web/src/uiStateStore.test.ts index faf7e62774d..9014fbe645d 100644 --- a/apps/web/src/uiStateStore.test.ts +++ b/apps/web/src/uiStateStore.test.ts @@ -270,6 +270,31 @@ describe("uiStateStore pure functions", () => { expect(next.projectExpandedById[logicalKey]).toBe(false); }); + it("syncProjects preserves expand state when a project's logical key changes", () => { + // Example: late-arriving repo metadata flips grouping identity from the + // physical key to a canonical repository key. The row did not actually + // change, so the user's collapse choice must carry over. + const physicalKey = "env-local:/repo/project"; + const previousLogicalKey = physicalKey; + const nextLogicalKey = "repo-canonical-key"; + + const initial = syncProjects(makeUiState(), [ + { key: physicalKey, logicalKey: previousLogicalKey, cwd: "/repo/project" }, + ]); + + expect(initial.projectExpandedById[previousLogicalKey]).toBe(true); + + const afterCollapse = { + ...initial, + projectExpandedById: { [previousLogicalKey]: false }, + }; + const next = syncProjects(afterCollapse, [ + { key: physicalKey, logicalKey: nextLogicalKey, cwd: "/repo/project" }, + ]); + + expect(next.projectExpandedById[nextLogicalKey]).toBe(false); + }); + it("syncThreads prunes missing thread UI state", () => { const thread1 = ThreadId.make("thread-1"); const thread2 = ThreadId.make("thread-2"); diff --git a/apps/web/src/uiStateStore.ts b/apps/web/src/uiStateStore.ts index eab7a68fe1b..d76f49c040e 100644 --- a/apps/web/src/uiStateStore.ts +++ b/apps/web/src/uiStateStore.ts @@ -57,6 +57,7 @@ const persistedExpandedProjectCwds = new Set(); const persistedProjectOrderCwds: string[] = []; const currentProjectCwdById = new Map(); const currentProjectCwdsByLogicalKey = new Map(); +const currentLogicalKeyByPhysicalKey = new Map(); let legacyKeysCleanedUp = false; function readPersistedState(): UiState { @@ -212,9 +213,12 @@ function nestedBooleanRecordsEqual( export function syncProjects(state: UiState, projects: readonly SyncProjectInput[]): UiState { const previousProjectCwdById = new Map(currentProjectCwdById); + const previousLogicalKeyByPhysicalKey = new Map(currentLogicalKeyByPhysicalKey); currentProjectCwdById.clear(); + currentLogicalKeyByPhysicalKey.clear(); for (const project of projects) { currentProjectCwdById.set(project.key, project.cwd); + currentLogicalKeyByPhysicalKey.set(project.key, project.logicalKey); } currentProjectCwdsByLogicalKey.clear(); for (const project of projects) { @@ -227,6 +231,23 @@ export function syncProjects(state: UiState, projects: readonly SyncProjectInput currentProjectCwdsByLogicalKey.set(project.logicalKey, [project.cwd]); } } + // Build reverse map: for each new logical key, which previous logical keys + // did its member projects live under? Lets us preserve expand state when a + // project's logical key changes (e.g. late-arriving repo metadata flips the + // group identity). + const previousLogicalKeysByNewLogicalKey = new Map>(); + for (const project of projects) { + const previousLogicalKey = previousLogicalKeyByPhysicalKey.get(project.key); + if (!previousLogicalKey || previousLogicalKey === project.logicalKey) { + continue; + } + const set = previousLogicalKeysByNewLogicalKey.get(project.logicalKey); + if (set) { + set.add(previousLogicalKey); + } else { + previousLogicalKeysByNewLogicalKey.set(project.logicalKey, new Set([previousLogicalKey])); + } + } const cwdMappingChanged = previousProjectCwdById.size !== currentProjectCwdById.size || projects.some((project) => previousProjectCwdById.get(project.key) !== project.cwd); @@ -239,8 +260,21 @@ export function syncProjects(state: UiState, projects: readonly SyncProjectInput const mappedProjects = projects.map((project, index) => { if (!(project.logicalKey in nextExpandedById)) { const groupCwds = currentProjectCwdsByLogicalKey.get(project.logicalKey) ?? [project.cwd]; + const fallbackFromPreviousLogicalKey = (() => { + const previousKeys = previousLogicalKeysByNewLogicalKey.get(project.logicalKey); + if (!previousKeys) { + return undefined; + } + for (const previousKey of previousKeys) { + if (previousKey in previousExpandedById) { + return previousExpandedById[previousKey]; + } + } + return undefined; + })(); const expanded = previousExpandedById[project.logicalKey] ?? + fallbackFromPreviousLogicalKey ?? (persistedExpandedProjectCwds.size > 0 ? groupCwds.some((cwd) => persistedExpandedProjectCwds.has(cwd)) : true);