diff --git a/packages/app/src/storage/validated-persist-storage.test.ts b/packages/app/src/storage/validated-persist-storage.test.ts index 7255086e37..4f102761e9 100644 --- a/packages/app/src/storage/validated-persist-storage.test.ts +++ b/packages/app/src/storage/validated-persist-storage.test.ts @@ -63,3 +63,66 @@ describe("createValidatedPersistStorage", () => { expect(backing.values.has("settings")).toBe(false); }); }); + +it("skips repeated immutable states only when requested", async () => { + let writes = 0; + const backing: StateStorage = { + getItem: () => null, + setItem: () => { + writes += 1; + }, + removeItem: () => {}, + }; + const value = { state: { enabled: true }, version: 1 }; + const regular = createValidatedPersistStorage(backing, StateSchema); + await regular.setItem("regular", value); + await regular.setItem("regular", value); + expect(writes).toBe(2); + + const deduplicated = createValidatedPersistStorage(backing, StateSchema, { skipUnchanged: true }); + await deduplicated.setItem("first", value); + await deduplicated.setItem("first", { ...value }); + expect(writes).toBe(3); + await deduplicated.setItem("second", value); + expect(writes).toBe(4); + await deduplicated.setItem("first", { ...value, version: 2 }); + expect(writes).toBe(5); +}); + +it("retries an unchanged state after a failed save", async () => { + let attempts = 0; + const backing = new MemoryStorage(); + const storage = createValidatedPersistStorage( + { + getItem: (name) => backing.getItem(name), + setItem: (name, value) => { + attempts += 1; + if (attempts === 1) return Promise.reject(new Error("storage unavailable")); + backing.setItem(name, value); + }, + removeItem: (name) => backing.removeItem(name), + }, + StateSchema, + { skipUnchanged: true }, + ); + const value = { state: { enabled: true }, version: 1 }; + await expect(storage.setItem("layout", value)).rejects.toThrow("storage unavailable"); + await storage.setItem("layout", value); + expect(attempts).toBe(2); + expect(backing.getItem("layout")).toBe(JSON.stringify(value)); +}); + +it("saves the same state again after removal or rehydration clears invalid storage", async () => { + const backing = new MemoryStorage(); + const storage = createValidatedPersistStorage(backing, StateSchema, { skipUnchanged: true }); + const value = { state: { enabled: true }, version: 1 }; + await storage.setItem("layout", value); + await storage.removeItem("layout"); + await storage.setItem("layout", value); + expect(backing.getItem("layout")).toBe(JSON.stringify(value)); + + backing.values.set("layout", "{"); + await expect(storage.getItem("layout")).resolves.toBeNull(); + await storage.setItem("layout", value); + expect(backing.getItem("layout")).toBe(JSON.stringify(value)); +}); diff --git a/packages/app/src/storage/validated-persist-storage.ts b/packages/app/src/storage/validated-persist-storage.ts index 07459f8642..e9a438daf9 100644 --- a/packages/app/src/storage/validated-persist-storage.ts +++ b/packages/app/src/storage/validated-persist-storage.ts @@ -1,17 +1,22 @@ import { z } from "zod"; -import type { PersistStorage, StateStorage } from "zustand/middleware"; +import type { PersistStorage, StateStorage, StorageValue } from "zustand/middleware"; export function createValidatedPersistStorage( backingStorage: StateStorage, stateSchema: z.ZodType, + // Opt in only when callers preserve state references until values change. + options: { skipUnchanged?: boolean } = {}, ): PersistStorage { const envelopeSchema = z.strictObject({ state: stateSchema, version: z.number().int().nonnegative().optional(), }); + const accepted = new Map>(); + return { getItem: async (name) => { + accepted.delete(name); const raw = await backingStorage.getItem(name); if (raw === null) return null; @@ -31,13 +36,31 @@ export function createValidatedPersistStorage( return result.data; }, setItem: async (name, value) => { + const previous = accepted.get(name); + if ( + options.skipUnchanged && + previous && + previous.state === value.state && + previous.version === value.version + ) + return; const result = envelopeSchema.safeParse(value); if (!result.success) { + accepted.delete(name); await backingStorage.removeItem(name); return; } - await backingStorage.setItem(name, JSON.stringify(result.data)); + if (options.skipUnchanged) accepted.set(name, value); + try { + await backingStorage.setItem(name, JSON.stringify(result.data)); + } catch (error) { + if (accepted.get(name) === value) accepted.delete(name); + throw error; + } + }, + removeItem: (name) => { + accepted.delete(name); + return backingStorage.removeItem(name); }, - removeItem: (name) => backingStorage.removeItem(name), }; } diff --git a/packages/app/src/stores/workspace-layout-persistence.test.ts b/packages/app/src/stores/workspace-layout-persistence.test.ts new file mode 100644 index 0000000000..d386a40298 --- /dev/null +++ b/packages/app/src/stores/workspace-layout-persistence.test.ts @@ -0,0 +1,54 @@ +import { indexedDB, IDBKeyRange } from "fake-indexeddb"; +import { expect, it } from "vitest"; +import type { StateStorage } from "zustand/middleware"; +import { createWorkspaceLayoutStore } from "./workspace-layout-store"; + +Object.assign(globalThis, { indexedDB, IDBKeyRange }); + +it("skips unchanged reconciliations and immediately saves real layout changes", async () => { + const values = new Map(); + let writes = 0; + const storage: StateStorage = { + getItem: (name) => values.get(name) ?? null, + setItem: (name, value) => { + writes += 1; + values.set(name, value); + }, + removeItem: (name) => { + values.delete(name); + }, + }; + const store = createWorkspaceLayoutStore(undefined, storage); + await store.persist.rehydrate(); + const workspaceKey = "server:workspace"; + const openAgent = (agentId: string) => + store.getState().openTab({ + workspaceKey, + target: { kind: "agent", agentId }, + intent: "reveal", + }); + openAgent("agent-1"); + const layout = store.getState().layoutByWorkspace[workspaceKey]; + const writesBefore = writes; + expect(writesBefore).toBe(1); + for (let index = 0; index < 50; index += 1) { + store.getState().reconcileTabs(workspaceKey, { + agentsHydrated: true, + terminalsHydrated: true, + activeAgentIds: ["agent-1"], + autoOpenAgentIds: ["agent-1"], + knownTerminalIds: [], + standaloneTerminalIds: [], + }); + } + expect(store.getState().layoutByWorkspace[workspaceKey]).toBe(layout); + expect(writes).toBe(writesBefore); + + openAgent("agent-2"); + expect(writes).toBe(writesBefore + 1); + const restored = createWorkspaceLayoutStore(undefined, storage); + await restored.persist.rehydrate(); + expect(restored.getState().layoutByWorkspace[workspaceKey]).toEqual( + store.getState().layoutByWorkspace[workspaceKey], + ); +}); diff --git a/packages/app/src/stores/workspace-layout-store.ts b/packages/app/src/stores/workspace-layout-store.ts index 2aea369df6..4bf86ae093 100644 --- a/packages/app/src/stores/workspace-layout-store.ts +++ b/packages/app/src/stores/workspace-layout-store.ts @@ -1,7 +1,7 @@ import AsyncStorage from "@react-native-async-storage/async-storage"; import { useEffect, useState } from "react"; import { create } from "zustand"; -import { persist } from "zustand/middleware"; +import { persist, type StateStorage } from "zustand/middleware"; import type { z } from "zod"; import { WorkspaceLayoutPersistedStateSchema } from "./workspace-layout-storage"; import type { JsonValue } from "@getpaseo/protocol/agent-types"; @@ -224,6 +224,7 @@ const MAX_TREE_DEPTH = 5; const LEGACY_EXPLORER_SIDEBAR_REFERENCE_WIDTH = 1440; const WORKSPACE_LAYOUT_PERSIST_VERSION = 2; +type WorkspaceLayoutPersistedState = z.infer; function convertLegacyExplorerSidebarRatios( ratiosByWorkspace: Record, @@ -939,8 +940,57 @@ function createExplorerSidebarPane( return targetPaneId ? splitPaneEmpty(workspaceKey, { targetPaneId, position: "right" }) : null; } +const PERSISTED_SLICES = [ + "layoutByWorkspace", + "pinnedAgentIdsByWorkspace", + "splitSizesByWorkspace", + "explorerSidebarWidthByWorkspace", + "explorerSidebarPaneIdByWorkspace", + "sidePaneIdByWorkspace", + "pullRequestTabAutoOpenedByWorkspace", +] as const; + +// Zustand persist re-runs partialize after every set, including ones that return the state +// untouched. Reusing the previous projection while every persisted slice is the same +// reference lets validated storage skip unchanged writes before validation and serialization. +function createPersistedStateProjection() { + let previous: { source: WorkspaceLayoutStore; state: WorkspaceLayoutPersistedState } | null = + null; + return (source: WorkspaceLayoutStore): WorkspaceLayoutPersistedState => { + const last = previous; + if (last && PERSISTED_SLICES.every((slice) => last.source[slice] === source[slice])) { + return last.state; + } + const layoutByWorkspace: Record = {}; + for (const key in source.layoutByWorkspace) { + // Strip ephemeral (commit diff) tabs before persisting so they are + // dropped on reload rather than restored pointing at a rebased SHA. + layoutByWorkspace[key] = stripEphemeralTabsFromLayout( + normalizeLayout(source.layoutByWorkspace[key]), + ); + } + const state: WorkspaceLayoutPersistedState = { + layoutByWorkspace, + pinnedAgentIdsByWorkspace: Object.fromEntries( + Object.entries(source.pinnedAgentIdsByWorkspace).map(([key, agentIds]) => [ + key, + Array.from(agentIds), + ]), + ), + splitSizesByWorkspace: source.splitSizesByWorkspace, + explorerSidebarWidthByWorkspace: source.explorerSidebarWidthByWorkspace, + explorerPaneIdByWorkspace: source.explorerSidebarPaneIdByWorkspace, + sidePaneIdByWorkspace: source.sidePaneIdByWorkspace, + pullRequestTabAutoOpenedByWorkspace: source.pullRequestTabAutoOpenedByWorkspace, + }; + previous = { source, state }; + return state; + }; +} + export function createWorkspaceLayoutStore( ids: WorkspaceLayoutIdSource = defaultWorkspaceLayoutIds, + storage: StateStorage = AsyncStorage, ) { return create()( persist( @@ -2117,33 +2167,12 @@ export function createWorkspaceLayoutStore( { name: "workspace-layout-state", version: WORKSPACE_LAYOUT_PERSIST_VERSION, - storage: createValidatedPersistStorage(AsyncStorage, WorkspaceLayoutPersistedStateSchema), + storage: createValidatedPersistStorage(storage, WorkspaceLayoutPersistedStateSchema, { + skipUnchanged: true, + }), migrate: (persistedState, version) => migrateWorkspaceLayoutPersistedState(persistedState, version, ids), - partialize: (state) => { - const layoutByWorkspace: Record = {}; - for (const key in state.layoutByWorkspace) { - // Strip ephemeral (commit diff) tabs before persisting so they are - // dropped on reload rather than restored pointing at a rebased SHA. - layoutByWorkspace[key] = stripEphemeralTabsFromLayout( - normalizeLayout(state.layoutByWorkspace[key]), - ); - } - return { - layoutByWorkspace, - pinnedAgentIdsByWorkspace: Object.fromEntries( - Object.entries(state.pinnedAgentIdsByWorkspace).map(([key, agentIds]) => [ - key, - Array.from(agentIds), - ]), - ), - splitSizesByWorkspace: state.splitSizesByWorkspace, - explorerSidebarWidthByWorkspace: state.explorerSidebarWidthByWorkspace, - explorerPaneIdByWorkspace: state.explorerSidebarPaneIdByWorkspace, - sidePaneIdByWorkspace: state.sidePaneIdByWorkspace, - pullRequestTabAutoOpenedByWorkspace: state.pullRequestTabAutoOpenedByWorkspace, - }; - }, + partialize: createPersistedStateProjection(), merge: (persistedState, currentState) => { const result = WorkspaceLayoutPersistedStateSchema.safeParse(persistedState); if (!result.success) {