diff --git a/apps/backend/internal/backendapp/boot_state_routes.go b/apps/backend/internal/backendapp/boot_state_routes.go index 300621780b..3525ce0bd6 100644 --- a/apps/backend/internal/backendapp/boot_state_routes.go +++ b/apps/backend/internal/backendapp/boot_state_routes.go @@ -462,6 +462,10 @@ func mapUserSettingsState(response userdto.UserSettingsResponse, workspaceID str "lspServerConfigs": mapStringMap(settings.LspServerConfigs), "savedLayouts": settings.SavedLayouts, "sidebarViews": mapSidebarViews(settings.SidebarViews), + "sidebarActiveViewId": nullString(settings.SidebarActiveViewID), + "sidebarDraft": mapSidebarDraft(settings.SidebarDraft), + "sidebarTaskPrefs": mapSidebarTaskPrefs(settings.SidebarTaskPrefs), + "taskCreateLastUsed": mapTaskCreateLastUsed(settings.TaskCreateLastUsed), "defaultUtilityAgentId": nullString(settings.DefaultUtilityAgentID), "keyboardShortcuts": mapStringAny(settings.KeyboardShortcuts), "terminalLinkBehavior": terminalLinkBehavior(settings.TerminalLinkBehavior), @@ -578,6 +582,36 @@ func mapSidebarViews(views []usermodels.SidebarView) []map[string]any { return result } +func mapSidebarDraft(draft *usermodels.SidebarViewDraft) map[string]any { + if draft == nil { + return nil + } + return map[string]any{ + "baseViewId": draft.BaseViewID, + "filters": draft.Filters, + "sort": draft.Sort, + "group": draft.Group, + } +} + +func mapSidebarTaskPrefs(prefs usermodels.SidebarTaskPrefs) map[string]any { + return map[string]any{ + "pinnedTaskIds": stringSlice(prefs.PinnedTaskIDs), + "orderedTaskIds": stringSlice(prefs.OrderedTaskIDs), + "subtaskOrderByParentId": stringSliceMap(prefs.SubtaskOrderByParentID), + } +} + +func mapTaskCreateLastUsed(value usermodels.TaskCreateLastUsed) map[string]any { + return map[string]any{ + "repositoryId": nullString(value.RepositoryID), + "branch": nullString(value.Branch), + "agentProfileId": nullString(value.AgentProfileID), + "executorProfileId": nullString(value.ExecutorProfileID), + "synced": value.RepositoryID != "" || value.Branch != "" || value.AgentProfileID != "" || value.ExecutorProfileID != "", + } +} + func mapVoiceMode(value usermodels.VoiceModeSettings) map[string]any { return map[string]any{ "enabled": value.Enabled, @@ -697,6 +731,13 @@ func stringSlice(value []string) []string { return value } +func stringSliceMap(value map[string][]string) map[string][]string { + if value == nil { + return map[string][]string{} + } + return value +} + func mapStringAny(value map[string]any) map[string]any { if value == nil { return map[string]any{} diff --git a/apps/backend/internal/backendapp/boot_state_routes_test.go b/apps/backend/internal/backendapp/boot_state_routes_test.go new file mode 100644 index 0000000000..eb3c60f3d4 --- /dev/null +++ b/apps/backend/internal/backendapp/boot_state_routes_test.go @@ -0,0 +1,63 @@ +package backendapp + +import ( + "testing" + + userdto "github.com/kandev/kandev/internal/user/dto" + usermodels "github.com/kandev/kandev/internal/user/models" +) + +func TestMapUserSettingsStateIncludesPortableTaskAndSidebarSettings(t *testing.T) { + state := mapUserSettingsState(userdto.UserSettingsResponse{ + Settings: userdto.UserSettingsDTO{ + SidebarViews: []usermodels.SidebarView{{ + ID: "view-1", + Name: "My view", + }}, + SidebarActiveViewID: "view-1", + SidebarDraft: &usermodels.SidebarViewDraft{ + BaseViewID: "view-1", + Group: "repository", + }, + SidebarTaskPrefs: usermodels.SidebarTaskPrefs{ + PinnedTaskIDs: []string{"task-1"}, + OrderedTaskIDs: []string{"task-2"}, + SubtaskOrderByParentID: map[string][]string{"task-1": {"task-3"}}, + }, + TaskCreateLastUsed: usermodels.TaskCreateLastUsed{ + RepositoryID: "repo-1", + Branch: "main", + AgentProfileID: "agent-1", + ExecutorProfileID: "executor-1", + }, + }, + }, "workspace-1") + + if state["sidebarActiveViewId"] != "view-1" { + t.Fatalf("sidebarActiveViewId = %#v, want view-1", state["sidebarActiveViewId"]) + } + draft, ok := state["sidebarDraft"].(map[string]any) + if !ok || draft["baseViewId"] != "view-1" || draft["group"] != "repository" { + t.Fatalf("sidebarDraft = %#v, want mapped draft", state["sidebarDraft"]) + } + prefs, ok := state["sidebarTaskPrefs"].(map[string]any) + if !ok || len(prefs["pinnedTaskIds"].([]string)) != 1 { + t.Fatalf("sidebarTaskPrefs = %#v, want mapped preferences", state["sidebarTaskPrefs"]) + } + lastUsed, ok := state["taskCreateLastUsed"].(map[string]any) + if !ok || lastUsed["repositoryId"] != "repo-1" || lastUsed["synced"] != true { + t.Fatalf("taskCreateLastUsed = %#v, want mapped settings", state["taskCreateLastUsed"]) + } +} + +func TestMapUserSettingsStateNormalizesNilSubtaskOrder(t *testing.T) { + state := mapUserSettingsState(userdto.UserSettingsResponse{}, "workspace-1") + prefs, ok := state["sidebarTaskPrefs"].(map[string]any) + if !ok { + t.Fatalf("sidebarTaskPrefs = %#v, want map[string]any", state["sidebarTaskPrefs"]) + } + order, ok := prefs["subtaskOrderByParentId"].(map[string][]string) + if !ok || order == nil || len(order) != 0 { + t.Fatalf("subtaskOrderByParentId = %#v, want empty map", prefs["subtaskOrderByParentId"]) + } +} diff --git a/apps/backend/internal/gateway/websocket/hub.go b/apps/backend/internal/gateway/websocket/hub.go index b215695feb..cb1e86fc53 100644 --- a/apps/backend/internal/gateway/websocket/hub.go +++ b/apps/backend/internal/gateway/websocket/hub.go @@ -30,6 +30,10 @@ type Hub struct { // Clients subscribed to backend/resource metrics. systemMetricsSubscribers map[*Client]bool + // done closes after Run has stopped so late connection teardown cannot + // block trying to unregister from an unserviced hub channel. + done chan struct{} + // Channels for client management register chan *Client unregister chan *Client @@ -68,6 +72,7 @@ func NewHub(dispatcher *ws.Dispatcher, log *logger.Logger) *Hub { userSubscribers: make(map[string]map[*Client]bool), runSubscribers: make(map[string]map[*Client]bool), systemMetricsSubscribers: make(map[*Client]bool), + done: make(chan struct{}), register: make(chan *Client), unregister: make(chan *Client), broadcast: make(chan *ws.Message, 256), @@ -85,7 +90,10 @@ type SystemMetricsInterestTracker interface { // Run starts the hub's main processing loop func (h *Hub) Run(ctx context.Context) { h.logger.Info("WebSocket hub started") - defer h.logger.Info("WebSocket hub stopped") + defer func() { + close(h.done) + h.logger.Info("WebSocket hub stopped") + }() h.mu.Lock() h.dispatchCtx = ctx @@ -254,12 +262,20 @@ func (h *Hub) broadcastMessage(msg *ws.Message) { // Register adds a client to the hub func (h *Hub) Register(client *Client) { - h.register <- client + select { + case h.register <- client: + case <-h.done: + client.closeSend() + } } // Unregister removes a client from the hub func (h *Hub) Unregister(client *Client) { - h.unregister <- client + select { + case h.unregister <- client: + case <-h.done: + client.closeSend() + } } // Broadcast sends a notification to all connected clients diff --git a/apps/backend/internal/gateway/websocket/hub_lifecycle_test.go b/apps/backend/internal/gateway/websocket/hub_lifecycle_test.go new file mode 100644 index 0000000000..3fab84ae83 --- /dev/null +++ b/apps/backend/internal/gateway/websocket/hub_lifecycle_test.go @@ -0,0 +1,42 @@ +package websocket + +import ( + "context" + "testing" + "time" +) + +func TestHub_ClientLifecycleCallsReturnAfterShutdown(t *testing.T) { + h := newTestHub(t) + ctx, cancel := context.WithCancel(context.Background()) + hubDone := make(chan struct{}) + go func() { + defer close(hubDone) + h.Run(ctx) + }() + cancel() + <-hubDone + + for name, lifecycleCall := range map[string]func(*Client){ + "register": h.Register, + "unregister": h.Unregister, + } { + t.Run(name, func(t *testing.T) { + client := newTestClient(name) + returned := make(chan struct{}) + go func() { + lifecycleCall(client) + close(returned) + }() + + select { + case <-returned: + case <-time.After(100 * time.Millisecond): + t.Fatal("client lifecycle call blocked after hub shutdown") + } + if !client.closed { + t.Fatal("client send channel remained open after hub shutdown") + } + }) + } +} diff --git a/apps/desktop/src-tauri/Cargo.lock b/apps/desktop/src-tauri/Cargo.lock index 1d7bb8711f..cf047edc2a 100644 --- a/apps/desktop/src-tauri/Cargo.lock +++ b/apps/desktop/src-tauri/Cargo.lock @@ -1876,7 +1876,6 @@ dependencies = [ "idna_adapter", "libc", "minisign-verify", - "notify-rust", "plist", "serde", "serde_json", diff --git a/apps/web/components/github/my-github/use-default-query-presets.test.ts b/apps/web/components/github/my-github/use-default-query-presets.test.ts index fe3fdb6e38..11f8e5eb91 100644 --- a/apps/web/components/github/my-github/use-default-query-presets.test.ts +++ b/apps/web/components/github/my-github/use-default-query-presets.test.ts @@ -12,7 +12,6 @@ import { } from "./use-default-query-presets"; const STORAGE_KEY = "kandev:github-default-queries:v1"; -const SYNC_FAILED_KEY = "kandev:github-default-queries:sync-failed:v1"; const WORKSPACE_ID = "ws-1"; const SETTINGS_TIMESTAMP = "2026-01-01T00:00:00Z"; @@ -82,88 +81,18 @@ function resetMocks() { ); } -describe("useDefaultQueryPresets legacy sync", () => { - beforeEach(resetMocks); - - it("retries local defaults after a failed backend sync and clears the marker", async () => { - const local = { pr: [preset], issue: [] }; - localStorageMock.setItem(STORAGE_KEY, JSON.stringify(local)); - localStorageMock.setItem(SYNC_FAILED_KEY, "1"); - - renderHook(() => useDefaultQueryPresets()); - - await waitFor(() => { - expect(updateUserSettings).toHaveBeenCalledWith({ - github_default_query_presets: local, - }); - expect(localStorageMock.getItem(SYNC_FAILED_KEY)).toBeNull(); - }); - }); - - it("retries null local defaults after a failed reset sync", async () => { - localStorageMock.setItem(SYNC_FAILED_KEY, "1"); - vi.mocked(fetchUserSettings).mockResolvedValue({ - settings: { github_default_query_presets: { pr: [preset], issue: [] } }, - } as Awaited>); - - renderHook(() => useDefaultQueryPresets()); - - await waitFor(() => { - expect(updateUserSettings).toHaveBeenCalledWith({ - github_default_query_presets: null, - }); - expect(localStorageMock.getItem(SYNC_FAILED_KEY)).toBeNull(); - }); - }); - - it("does not overwrite cross-tab local changes with a stale hydration response", async () => { - const initial = { pr: [preset], issue: [] }; - const crossTab = { pr: [], issue: [preset] }; - const server = { - pr: [{ ...preset, value: "server", label: "Server" }], - issue: [], - }; - let resolveFetch: (value: Awaited>) => void = () => {}; - vi.mocked(fetchUserSettings).mockReturnValue( - new Promise((resolve) => { - resolveFetch = resolve; - }), - ); - localStorageMock.setItem(STORAGE_KEY, JSON.stringify(initial)); - - renderHook(() => useDefaultQueryPresets()); - - await waitFor(() => expect(fetchUserSettings).toHaveBeenCalled()); - localStorageMock.setItem(STORAGE_KEY, JSON.stringify(crossTab)); - const event = new Event("storage"); - Object.defineProperty(event, "key", { value: STORAGE_KEY }); - window.dispatchEvent(event); - resolveFetch({ - settings: { github_default_query_presets: server }, - } as Awaited>); - await Promise.resolve(); - await Promise.resolve(); - - expect(localStorageMock.getItem(STORAGE_KEY)).toBe(JSON.stringify(crossTab)); - expect(updateUserSettings).not.toHaveBeenCalled(); - }); -}); - describe("useDefaultQueryPresets workspace sync", () => { beforeEach(resetMocks); - it("migrates local defaults into a fresh workspace", async () => { + it("ignores stale local defaults when workspace settings are empty", async () => { const local = { pr: [preset], issue: [] }; localStorageMock.setItem(STORAGE_KEY, JSON.stringify(local)); __resetSnapshotForTests(); const { result } = renderHook(() => useDefaultQueryPresets(WORKSPACE_ID)); - await waitFor(() => expect(result.current.prPresets[0]?.value).toBe("mine")); - expect(updateGitHubWorkspaceSettings).toHaveBeenCalledWith({ - workspace_id: WORKSPACE_ID, - default_query_presets: local, - }); + await waitFor(() => expect(result.current.prPresets[0]?.value).not.toBe("mine")); + expect(updateGitHubWorkspaceSettings).not.toHaveBeenCalled(); }); it("does not migrate local defaults over existing workspace defaults", async () => { diff --git a/apps/web/components/github/my-github/use-default-query-presets.ts b/apps/web/components/github/my-github/use-default-query-presets.ts index 73d1b1cef8..c0289f05c4 100644 --- a/apps/web/components/github/my-github/use-default-query-presets.ts +++ b/apps/web/components/github/my-github/use-default-query-presets.ts @@ -12,12 +12,6 @@ import { updateGitHubWorkspaceSettings, } from "@/lib/api/domains/github-api"; import { createQueuedUserSettingsSync } from "@/lib/user-settings-sync"; -import { hasUserSettingsSyncFailure } from "@/lib/user-settings-sync-failure"; - -const STORAGE_KEY = "kandev:github-default-queries:v1"; -const MIGRATED_KEY = "kandev:github-default-queries:migrated-to-backend:v1"; -const WORKSPACE_MIGRATED_KEY_PREFIX = "kandev:github-default-queries:migrated-to-workspace:v1:"; -const SYNC_FAILED_KEY = "kandev:github-default-queries:sync-failed:v1"; export type StoredQueryPreset = { value: string; @@ -35,45 +29,13 @@ export function toStored(presets: PresetOption[]): StoredQueryPreset[] { return presets.map(({ value, label, filter, group }) => ({ value, label, filter, group })); } -function readStorage(): StoredDefaults | null { - if (typeof window === "undefined") return null; - try { - const raw = window.localStorage.getItem(STORAGE_KEY); - if (!raw) return null; - const parsed = JSON.parse(raw) as unknown; - if ( - typeof parsed !== "object" || - parsed === null || - !Array.isArray((parsed as StoredDefaults).pr) || - !Array.isArray((parsed as StoredDefaults).issue) - ) { - return null; - } - return parsed as StoredDefaults; - } catch { - return null; - } -} - -function writeStorage(defaults: StoredDefaults | null) { - if (typeof window === "undefined") return; - try { - if (defaults === null) { - window.localStorage.removeItem(STORAGE_KEY); - } else { - window.localStorage.setItem(STORAGE_KEY, JSON.stringify(defaults)); - } - } catch { - /* ignore quota / access errors */ - } -} - let snapshot: StoredDefaults | null | undefined = undefined; +let snapshotVersion = 0; const listeners = new Set<() => void>(); function publish(next: StoredDefaults | null) { snapshot = next; - writeStorage(next); + snapshotVersion += 1; listeners.forEach((fn) => fn()); } @@ -89,22 +51,9 @@ function readServerDefaults(value: unknown): StoredDefaults | null | undefined { return value as StoredDefaults; } -async function readLegacyServerDefaults(): Promise { - try { - const response = await fetchUserSettings({ cache: "no-store" }); - const defaults = readServerDefaults(response.settings.github_default_query_presets); - return defaults === undefined ? null : defaults; - } catch { - return undefined; - } -} - -const syncServer = createQueuedUserSettingsSync( - SYNC_FAILED_KEY, - (defaults) => ({ - github_default_query_presets: defaults, - }), -); +const syncServer = createQueuedUserSettingsSync((defaults) => ({ + github_default_query_presets: defaults, +})); let workspaceSyncQueue = Promise.resolve(); @@ -123,55 +72,15 @@ function syncWorkspaceDefaultQueryPresets( return workspaceSyncQueue; } -function snapshotKey(value: StoredDefaults | null): string { - return JSON.stringify(value); -} - -function hasMigratedToBackend(): boolean { - if (typeof window === "undefined") return false; - return window.localStorage.getItem(MIGRATED_KEY) === "1"; -} - -function markMigratedToBackend(): void { - if (typeof window === "undefined") return; - try { - window.localStorage.setItem(MIGRATED_KEY, "1"); - } catch { - /* ignore storage failures */ - } -} - -function hasMigratedToWorkspace(workspaceId: string): boolean { - if (typeof window === "undefined") return false; - return window.localStorage.getItem(WORKSPACE_MIGRATED_KEY_PREFIX + workspaceId) === "1"; -} - -function markMigratedToWorkspace(workspaceId: string): void { - if (typeof window === "undefined") return; - try { - window.localStorage.setItem(WORKSPACE_MIGRATED_KEY_PREFIX + workspaceId, "1"); - } catch { - /* ignore storage failures */ - } -} - function subscribe(cb: () => void) { listeners.add(cb); - const onStorage = (event: StorageEvent) => { - if (event.key !== STORAGE_KEY) return; - snapshot = readStorage(); - listeners.forEach((fn) => fn()); - }; - window.addEventListener("storage", onStorage); return () => { listeners.delete(cb); - window.removeEventListener("storage", onStorage); }; } function getSnapshot(): StoredDefaults | null { - if (snapshot === undefined) snapshot = readStorage(); - return snapshot; + return snapshot ?? null; } function getServerSnapshot(): StoredDefaults | null { @@ -180,31 +89,20 @@ function getServerSnapshot(): StoredDefaults | null { export function __resetSnapshotForTests() { snapshot = undefined; + snapshotVersion = 0; listeners.forEach((fn) => fn()); } -function useLegacyDefaultQueryPresetSync(enabled: boolean) { +function useUserDefaultQueryPresetSync(enabled: boolean) { useEffect(() => { if (!enabled) return; let cancelled = false; - const initialKey = snapshotKey(getSnapshot()); + const initialVersion = snapshotVersion; fetchUserSettings({ cache: "no-store" }) .then((response) => { const serverDefaults = readServerDefaults(response.settings.github_default_query_presets); - if (cancelled || serverDefaults === undefined) return; - const local = getSnapshot(); - if (snapshotKey(local) !== initialKey) return; - if (hasUserSettingsSyncFailure(SYNC_FAILED_KEY)) { - void syncServer(local); - return; - } - if (serverDefaults === null && local !== null && !hasMigratedToBackend()) { - void syncServer(local); - markMigratedToBackend(); - return; - } + if (cancelled || serverDefaults === undefined || snapshotVersion !== initialVersion) return; publish(serverDefaults); - markMigratedToBackend(); }) .catch(() => {}); return () => { @@ -227,30 +125,10 @@ function useWorkspaceDefaultQueryPresets(workspaceId: string | null) { const seq = writeSeq.current; setWorkspaceDefaults(undefined); fetchGitHubWorkspaceSettings(workspaceId) - .then(async (settings) => { + .then((settings) => { if (cancelled || seq !== writeSeq.current) return; const defaults = readServerDefaults(settings.default_query_presets); - let local: StoredDefaults | null | undefined = getSnapshot(); - if (local === null) { - local = await readLegacyServerDefaults(); - } - if (cancelled || seq !== writeSeq.current) return; - if ( - (defaults === null || defaults === undefined) && - local !== undefined && - local !== null && - !hasMigratedToWorkspace(workspaceId) - ) { - setWorkspaceDefaults(local); - void syncWorkspaceDefaultQueryPresets(workspaceId, local) - .then(() => markMigratedToWorkspace(workspaceId)) - .catch(() => {}); - return; - } setWorkspaceDefaults(defaults === undefined ? null : defaults); - if (!(local === undefined && (defaults === null || defaults === undefined))) { - markMigratedToWorkspace(workspaceId); - } }) .catch(() => { if (!cancelled) setWorkspaceDefaults(undefined); @@ -269,7 +147,7 @@ function useWorkspaceDefaultQueryPresets(workspaceId: string | null) { export function useDefaultQueryPresets(workspaceId: string | null = null) { const stored = useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot); const { workspaceDefaults, setWorkspaceDefaults } = useWorkspaceDefaultQueryPresets(workspaceId); - useLegacyDefaultQueryPresetSync(!workspaceId); + useUserDefaultQueryPresetSync(!workspaceId); const effectiveStored = workspaceId ? workspaceDefaults : stored; const prPresets = useMemo( () => effectiveStored?.pr ?? toStored(BUILTIN_PR_PRESETS), @@ -285,14 +163,11 @@ export function useDefaultQueryPresets(workspaceId: string | null = null) { if (workspaceId && workspaceDefaults === undefined) return; if (workspaceId) { setWorkspaceDefaults(defaults); - void syncWorkspaceDefaultQueryPresets(workspaceId, defaults) - .then(() => markMigratedToWorkspace(workspaceId)) - .catch(() => {}); + void syncWorkspaceDefaultQueryPresets(workspaceId, defaults).catch(() => {}); return; } publish(defaults); void syncServer(defaults); - markMigratedToBackend(); }, [workspaceId, workspaceDefaults, setWorkspaceDefaults], ); @@ -301,14 +176,11 @@ export function useDefaultQueryPresets(workspaceId: string | null = null) { if (workspaceId && workspaceDefaults === undefined) return; if (workspaceId) { setWorkspaceDefaults(null); - void syncWorkspaceDefaultQueryPresets(workspaceId, null) - .then(() => markMigratedToWorkspace(workspaceId)) - .catch(() => {}); + void syncWorkspaceDefaultQueryPresets(workspaceId, null).catch(() => {}); return; } publish(null); void syncServer(null); - markMigratedToBackend(); }, [workspaceId, workspaceDefaults, setWorkspaceDefaults]); const isCustomized = effectiveStored !== null && effectiveStored !== undefined; diff --git a/apps/web/components/github/my-github/use-saved-presets.test.ts b/apps/web/components/github/my-github/use-saved-presets.test.ts index eb18f85ea6..df72e038f9 100644 --- a/apps/web/components/github/my-github/use-saved-presets.test.ts +++ b/apps/web/components/github/my-github/use-saved-presets.test.ts @@ -5,15 +5,9 @@ import { fetchGitHubWorkspaceSettings, updateGitHubWorkspaceSettings, } from "@/lib/api/domains/github-api"; -import { - __resetSnapshotForTests, - readStorage, - useSavedPresets, - type SavedPreset, -} from "./use-saved-presets"; +import { __resetSnapshotForTests, useSavedPresets, type SavedPreset } from "./use-saved-presets"; const STORAGE_KEY = "kandev:github-presets:v1"; -const SYNC_FAILED_KEY = "kandev:github-presets:sync-failed:v1"; const WORKSPACE_ID = "ws-1"; const SETTINGS_TIMESTAMP = "2026-01-01T00:00:00Z"; @@ -27,30 +21,9 @@ vi.mock("@/lib/api/domains/github-api", () => ({ updateGitHubWorkspaceSettings: vi.fn(), })); -// Provide a simple in-memory localStorage mock so the tests are not sensitive -// to how the test runner exposes window.localStorage (e.g. Node's -// --localstorage-file flag without a valid path). -function makeLocalStorageMock() { - const store = new Map(); - return { - getItem: (key: string) => store.get(key) ?? null, - setItem: (key: string, value: string) => store.set(key, value), - removeItem: (key: string) => store.delete(key), - clear: () => store.clear(), - get length() { - return store.size; - }, - key: (index: number) => Array.from(store.keys())[index] ?? null, - }; -} - -const localStorageMock = makeLocalStorageMock(); -vi.stubGlobal("localStorage", localStorageMock); -Object.defineProperty(window, "localStorage", { value: localStorageMock, configurable: true }); - function set(raw: string | null) { - if (raw === null) localStorageMock.removeItem(STORAGE_KEY); - else localStorageMock.setItem(STORAGE_KEY, raw); + if (raw === null) window.localStorage.removeItem(STORAGE_KEY); + else window.localStorage.setItem(STORAGE_KEY, raw); } const valid: SavedPreset = { @@ -63,7 +36,7 @@ const valid: SavedPreset = { }; function resetTestState() { - localStorageMock.clear(); + window.localStorage.clear(); __resetSnapshotForTests(); vi.mocked(fetchUserSettings).mockReset(); vi.mocked(updateUserSettings).mockReset(); @@ -86,98 +59,21 @@ function workspaceSettings( } as Awaited>; } -describe("readStorage", () => { - beforeEach(() => { - resetTestState(); - }); - - it("returns empty array when no value is stored", () => { - expect(readStorage()).toEqual([]); - }); - - it("returns empty array for malformed JSON", () => { - set("not-json{"); - expect(readStorage()).toEqual([]); - }); - - it("returns empty array when parsed value is not an array", () => { - set(JSON.stringify({ id: "p_1" })); - expect(readStorage()).toEqual([]); - }); - - it("keeps valid entries", () => { - set(JSON.stringify([valid])); - expect(readStorage()).toEqual([valid]); - }); - - it("drops entries missing an id", () => { - const missingId = { ...valid } as Partial; - delete missingId.id; - set(JSON.stringify([missingId, valid])); - expect(readStorage()).toEqual([valid]); - }); - - it("drops entries with invalid kind", () => { - set(JSON.stringify([{ ...valid, kind: "commit" }, valid])); - expect(readStorage()).toEqual([valid]); - }); - - it("drops non-object entries", () => { - set(JSON.stringify(["string", 42, null, valid])); - expect(readStorage()).toEqual([valid]); - }); - - it("drops entries with non-string label", () => { - set(JSON.stringify([{ ...valid, label: 123 }, valid])); - expect(readStorage()).toEqual([valid]); - }); - - it("accepts issue kind", () => { - const issue: SavedPreset = { ...valid, kind: "issue" }; - set(JSON.stringify([issue])); - expect(readStorage()).toEqual([issue]); - }); -}); - describe("useSavedPresets", () => { beforeEach(() => { resetTestState(); }); - it("retries local presets after a failed backend sync and clears the marker", async () => { + it("ignores stale local presets when backend settings are empty", async () => { set(JSON.stringify([valid])); - localStorageMock.setItem(SYNC_FAILED_KEY, "1"); vi.mocked(fetchUserSettings).mockResolvedValue({ settings: { github_saved_presets: [] }, } as Awaited>); - vi.mocked(updateUserSettings).mockResolvedValue({ - settings: {}, - } as Awaited>); - renderHook(() => useSavedPresets()); + const { result } = renderHook(() => useSavedPresets()); - await waitFor(() => { - expect(updateUserSettings).toHaveBeenCalledWith({ github_saved_presets: [valid] }); - expect(localStorageMock.getItem(SYNC_FAILED_KEY)).toBeNull(); - }); - }); - - it("retries empty local presets after a failed backend sync", async () => { - set(JSON.stringify([])); - localStorageMock.setItem(SYNC_FAILED_KEY, "1"); - vi.mocked(fetchUserSettings).mockResolvedValue({ - settings: { github_saved_presets: [valid] }, - } as Awaited>); - vi.mocked(updateUserSettings).mockResolvedValue({ - settings: {}, - } as Awaited>); - - renderHook(() => useSavedPresets()); - - await waitFor(() => { - expect(updateUserSettings).toHaveBeenCalledWith({ github_saved_presets: [] }); - expect(localStorageMock.getItem(SYNC_FAILED_KEY)).toBeNull(); - }); + await waitFor(() => expect(result.current.presets).toEqual([])); + expect(updateUserSettings).not.toHaveBeenCalled(); }); }); @@ -186,20 +82,14 @@ describe("useSavedPresets workspace sync", () => { resetTestState(); }); - it("migrates local presets into a fresh workspace", async () => { + it("ignores stale local presets when workspace settings are empty", async () => { set(JSON.stringify([valid])); vi.mocked(fetchGitHubWorkspaceSettings).mockResolvedValue(workspaceSettings()); - vi.mocked(updateGitHubWorkspaceSettings).mockResolvedValue( - {} as Awaited>, - ); const { result } = renderHook(() => useSavedPresets(WORKSPACE_ID)); - await waitFor(() => expect(result.current.presets).toEqual([valid])); - expect(updateGitHubWorkspaceSettings).toHaveBeenCalledWith({ - workspace_id: WORKSPACE_ID, - saved_presets: [valid], - }); + await waitFor(() => expect(result.current.presets).toEqual([])); + expect(updateGitHubWorkspaceSettings).not.toHaveBeenCalled(); }); it("does not migrate local presets over existing workspace presets", async () => { diff --git a/apps/web/components/github/my-github/use-saved-presets.ts b/apps/web/components/github/my-github/use-saved-presets.ts index cbe495086f..8dd7cb3ec1 100644 --- a/apps/web/components/github/my-github/use-saved-presets.ts +++ b/apps/web/components/github/my-github/use-saved-presets.ts @@ -7,12 +7,6 @@ import { updateGitHubWorkspaceSettings, } from "@/lib/api/domains/github-api"; import { createQueuedUserSettingsSync } from "@/lib/user-settings-sync"; -import { hasUserSettingsSyncFailure } from "@/lib/user-settings-sync-failure"; - -const STORAGE_KEY = "kandev:github-presets:v1"; -const MIGRATED_KEY = "kandev:github-presets:migrated-to-backend:v1"; -const WORKSPACE_MIGRATED_KEY_PREFIX = "kandev:github-presets:migrated-to-workspace:v1:"; -const SYNC_FAILED_KEY = "kandev:github-presets:sync-failed:v1"; export type SavedPreset = { id: string; @@ -23,42 +17,12 @@ export type SavedPreset = { createdAt: string; }; -export function readStorage(): SavedPreset[] { - if (typeof window === "undefined") return []; - try { - const raw = window.localStorage.getItem(STORAGE_KEY); - if (!raw) return []; - const parsed = JSON.parse(raw) as unknown; - if (!Array.isArray(parsed)) return []; - return parsed.filter( - (p): p is SavedPreset => - typeof p === "object" && - p !== null && - typeof (p as SavedPreset).id === "string" && - ((p as SavedPreset).kind === "pr" || (p as SavedPreset).kind === "issue") && - typeof (p as SavedPreset).label === "string", - ); - } catch { - return []; - } -} - -function writeStorage(presets: SavedPreset[]) { - if (typeof window === "undefined") return; - try { - window.localStorage.setItem(STORAGE_KEY, JSON.stringify(presets)); - } catch { - /* ignore quota / access errors */ - } -} - -// External store: single source of truth shared across all hook consumers. const listeners = new Set<() => void>(); -let snapshot: SavedPreset[] | null = null; +let snapshot: SavedPreset[] = []; +let snapshotVersion = 0; const emptySnapshot: SavedPreset[] = []; function getSnapshot(): SavedPreset[] { - if (snapshot === null) snapshot = readStorage(); return snapshot; } @@ -75,7 +39,7 @@ function subscribe(cb: () => void) { function publish(next: SavedPreset[]) { snapshot = next; - writeStorage(next); + snapshotVersion += 1; for (const l of listeners) l(); } @@ -91,16 +55,7 @@ function readServerPresets(value: unknown): SavedPreset[] | null { ); } -async function readLegacyServerPresets(): Promise { - try { - const response = await fetchUserSettings({ cache: "no-store" }); - return readServerPresets(response.settings.github_saved_presets) ?? []; - } catch { - return undefined; - } -} - -const syncServer = createQueuedUserSettingsSync(SYNC_FAILED_KEY, (next) => ({ +const syncServer = createQueuedUserSettingsSync((next) => ({ github_saved_presets: next, })); @@ -118,65 +73,22 @@ function syncWorkspaceSavedPresets(workspaceId: string, next: SavedPreset[]): Pr return workspaceSyncQueue; } -function snapshotKey(value: SavedPreset[]): string { - return JSON.stringify(value); -} - -function hasMigratedToBackend(): boolean { - if (typeof window === "undefined") return false; - return window.localStorage.getItem(MIGRATED_KEY) === "1"; -} - -function markMigratedToBackend(): void { - if (typeof window === "undefined") return; - try { - window.localStorage.setItem(MIGRATED_KEY, "1"); - } catch { - /* ignore storage failures */ - } -} - -function hasMigratedToWorkspace(workspaceId: string): boolean { - if (typeof window === "undefined") return false; - return window.localStorage.getItem(WORKSPACE_MIGRATED_KEY_PREFIX + workspaceId) === "1"; -} - -function markMigratedToWorkspace(workspaceId: string): void { - if (typeof window === "undefined") return; - try { - window.localStorage.setItem(WORKSPACE_MIGRATED_KEY_PREFIX + workspaceId, "1"); - } catch { - /* ignore storage failures */ - } -} - export function __resetSnapshotForTests() { - snapshot = null; + snapshot = []; + snapshotVersion = 0; for (const l of listeners) l(); } -function useLegacySavedPresetsSync(enabled: boolean) { +function useUserSavedPresetsSync(enabled: boolean) { useEffect(() => { if (!enabled) return; let cancelled = false; - const initialKey = snapshotKey(readStorage()); + const initialVersion = snapshotVersion; fetchUserSettings({ cache: "no-store" }) .then((response) => { const serverPresets = readServerPresets(response.settings.github_saved_presets); - if (cancelled || !serverPresets) return; - const local = readStorage(); - if (snapshotKey(local) !== initialKey) return; - if (hasUserSettingsSyncFailure(SYNC_FAILED_KEY)) { - void syncServer(local); - return; - } - if (serverPresets.length === 0 && local.length > 0 && !hasMigratedToBackend()) { - void syncServer(local); - markMigratedToBackend(); - return; - } + if (cancelled || !serverPresets || snapshotVersion !== initialVersion) return; publish(serverPresets); - markMigratedToBackend(); }) .catch(() => {}); return () => { @@ -197,30 +109,10 @@ function useWorkspaceSavedPresets(workspaceId: string | null) { const seq = writeSeq.current; setWorkspacePresets(undefined); fetchGitHubWorkspaceSettings(workspaceId) - .then(async (settings) => { + .then((settings) => { if (cancelled || seq !== writeSeq.current) return; const serverPresets = readServerPresets(settings.saved_presets) ?? []; - let local: SavedPreset[] | undefined = readStorage(); - if (local.length === 0) { - local = await readLegacyServerPresets(); - } - if (cancelled || seq !== writeSeq.current) return; - if ( - serverPresets.length === 0 && - local !== undefined && - local.length > 0 && - !hasMigratedToWorkspace(workspaceId) - ) { - setWorkspacePresets(local); - void syncWorkspaceSavedPresets(workspaceId, local) - .then(() => markMigratedToWorkspace(workspaceId)) - .catch(() => {}); - return; - } setWorkspacePresets(serverPresets); - if (local !== undefined || serverPresets.length > 0) { - markMigratedToWorkspace(workspaceId); - } }) .catch(() => { if (!cancelled) setWorkspacePresets(undefined); @@ -239,7 +131,7 @@ function useWorkspaceSavedPresets(workspaceId: string | null) { export function useSavedPresets(workspaceId: string | null = null) { const presets = useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot); const { workspacePresets, setWorkspacePresets } = useWorkspaceSavedPresets(workspaceId); - useLegacySavedPresetsSync(!workspaceId); + useUserSavedPresetsSync(!workspaceId); const activePresets = workspaceId ? (workspacePresets ?? []) : presets; const save = useCallback( @@ -253,14 +145,11 @@ export function useSavedPresets(workspaceId: string | null = null) { const next = [...activePresets, preset]; if (workspaceId) { setWorkspacePresets(next); - void syncWorkspaceSavedPresets(workspaceId, next) - .then(() => markMigratedToWorkspace(workspaceId)) - .catch(() => {}); + void syncWorkspaceSavedPresets(workspaceId, next).catch(() => {}); return preset; } publish(next); void syncServer(next); - markMigratedToBackend(); return preset; }, [activePresets, workspaceId, workspacePresets, setWorkspacePresets], @@ -272,14 +161,11 @@ export function useSavedPresets(workspaceId: string | null = null) { const next = activePresets.filter((p) => p.id !== id); if (workspaceId) { setWorkspacePresets(next); - void syncWorkspaceSavedPresets(workspaceId, next) - .then(() => markMigratedToWorkspace(workspaceId)) - .catch(() => {}); + void syncWorkspaceSavedPresets(workspaceId, next).catch(() => {}); return; } publish(next); void syncServer(next); - markMigratedToBackend(); }, [activePresets, workspaceId, workspacePresets, setWorkspacePresets], ); diff --git a/apps/web/components/gitlab/my-gitlab/use-saved-presets.test.ts b/apps/web/components/gitlab/my-gitlab/use-saved-presets.test.ts index 8b8a499ca2..d2c0e5fc93 100644 --- a/apps/web/components/gitlab/my-gitlab/use-saved-presets.test.ts +++ b/apps/web/components/gitlab/my-gitlab/use-saved-presets.test.ts @@ -1,41 +1,18 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { act, renderHook, waitFor } from "@testing-library/react"; import { fetchUserSettings, updateUserSettings } from "@/lib/api/domains/settings-api"; -import { - __resetSnapshotForTests, - readStorage, - useSavedPresets, - type SavedPreset, -} from "./use-saved-presets"; +import { __resetSnapshotForTests, useSavedPresets, type SavedPreset } from "./use-saved-presets"; const STORAGE_KEY = "kandev:gitlab-presets:v1"; -const SYNC_FAILED_KEY = "kandev:gitlab-presets:sync-failed:v1"; vi.mock("@/lib/api/domains/settings-api", () => ({ fetchUserSettings: vi.fn(), updateUserSettings: vi.fn(), })); -function makeLocalStorageMock() { - const store = new Map(); - return { - getItem: (key: string) => store.get(key) ?? null, - setItem: (key: string, value: string) => store.set(key, value), - removeItem: (key: string) => store.delete(key), - clear: () => store.clear(), - get length() { - return store.size; - }, - key: (index: number) => Array.from(store.keys())[index] ?? null, - }; -} - -const localStorageMock = makeLocalStorageMock(); -vi.stubGlobal("localStorage", localStorageMock); - function set(raw: string | null) { - if (raw === null) localStorageMock.removeItem(STORAGE_KEY); - else localStorageMock.setItem(STORAGE_KEY, raw); + if (raw === null) window.localStorage.removeItem(STORAGE_KEY); + else window.localStorage.setItem(STORAGE_KEY, raw); } const valid: SavedPreset = { @@ -47,83 +24,10 @@ const valid: SavedPreset = { createdAt: "2026-01-01T00:00:00Z", }; -describe("readStorage (gitlab)", () => { - beforeEach(() => { - localStorageMock.clear(); - }); - - it("returns empty array when no value is stored", () => { - expect(readStorage()).toEqual([]); - }); - - it("returns empty array for malformed JSON", () => { - set("not-json{"); - expect(readStorage()).toEqual([]); - }); - - it("returns empty array when parsed value is not an array", () => { - set(JSON.stringify({ id: "g_1" })); - expect(readStorage()).toEqual([]); - }); - - it("keeps valid entries", () => { - set(JSON.stringify([valid])); - expect(readStorage()).toEqual([valid]); - }); - - it("drops entries missing an id", () => { - const missingId = { ...valid } as Partial; - delete missingId.id; - set(JSON.stringify([missingId, valid])); - expect(readStorage()).toEqual([valid]); - }); - - it("drops entries with invalid kind", () => { - set(JSON.stringify([{ ...valid, kind: "pr" }, valid])); - expect(readStorage()).toEqual([valid]); - }); - - it("drops non-object entries", () => { - set(JSON.stringify(["string", 42, null, valid])); - expect(readStorage()).toEqual([valid]); - }); - - it("drops entries with non-string label", () => { - set(JSON.stringify([{ ...valid, label: 123 }, valid])); - expect(readStorage()).toEqual([valid]); - }); - - it("accepts issue kind", () => { - const issue: SavedPreset = { ...valid, kind: "issue" }; - set(JSON.stringify([issue])); - expect(readStorage()).toEqual([issue]); - }); - - it("drops entries missing customQuery", () => { - const broken = { ...valid } as Partial; - delete broken.customQuery; - set(JSON.stringify([broken, valid])); - expect(readStorage()).toEqual([valid]); - }); - - it("drops entries missing projectFilter", () => { - const broken = { ...valid } as Partial; - delete broken.projectFilter; - set(JSON.stringify([broken, valid])); - expect(readStorage()).toEqual([valid]); - }); - - it("drops entries missing createdAt", () => { - const broken = { ...valid } as Partial; - delete broken.createdAt; - set(JSON.stringify([broken, valid])); - expect(readStorage()).toEqual([valid]); - }); -}); - describe("useSavedPresets (gitlab)", () => { beforeEach(() => { - localStorageMock.clear(); + vi.clearAllMocks(); + window.localStorage.clear(); __resetSnapshotForTests(); vi.mocked(fetchUserSettings).mockResolvedValue({ settings: { gitlab_saved_presets: [] }, @@ -183,30 +87,13 @@ describe("useSavedPresets (gitlab)", () => { expect(b.result.current.presets[0].label).toBe("Shared"); }); - it("retries local presets after a failed backend sync and clears the marker", async () => { + it("ignores stale local presets when backend settings are empty", async () => { set(JSON.stringify([valid])); - localStorageMock.setItem(SYNC_FAILED_KEY, "1"); + __resetSnapshotForTests(); - renderHook(() => useSavedPresets()); + const { result } = renderHook(() => useSavedPresets()); - await waitFor(() => { - expect(updateUserSettings).toHaveBeenCalledWith({ gitlab_saved_presets: [valid] }); - expect(localStorageMock.getItem(SYNC_FAILED_KEY)).toBeNull(); - }); - }); - - it("retries empty local presets after a failed backend sync", async () => { - set(JSON.stringify([])); - localStorageMock.setItem(SYNC_FAILED_KEY, "1"); - vi.mocked(fetchUserSettings).mockResolvedValue({ - settings: { gitlab_saved_presets: [valid] }, - } as Awaited>); - - renderHook(() => useSavedPresets()); - - await waitFor(() => { - expect(updateUserSettings).toHaveBeenCalledWith({ gitlab_saved_presets: [] }); - expect(localStorageMock.getItem(SYNC_FAILED_KEY)).toBeNull(); - }); + await waitFor(() => expect(result.current.presets).toEqual([])); + expect(updateUserSettings).not.toHaveBeenCalled(); }); }); diff --git a/apps/web/components/gitlab/my-gitlab/use-saved-presets.ts b/apps/web/components/gitlab/my-gitlab/use-saved-presets.ts index 62003364f9..5355ae85f2 100644 --- a/apps/web/components/gitlab/my-gitlab/use-saved-presets.ts +++ b/apps/web/components/gitlab/my-gitlab/use-saved-presets.ts @@ -3,11 +3,6 @@ import { useCallback, useEffect, useSyncExternalStore } from "react"; import { fetchUserSettings } from "@/lib/api/domains/settings-api"; import { createQueuedUserSettingsSync } from "@/lib/user-settings-sync"; -import { hasUserSettingsSyncFailure } from "@/lib/user-settings-sync-failure"; - -const STORAGE_KEY = "kandev:gitlab-presets:v1"; -const MIGRATED_KEY = "kandev:gitlab-presets:migrated-to-backend:v1"; -const SYNC_FAILED_KEY = "kandev:gitlab-presets:sync-failed:v1"; export type SavedPreset = { id: string; @@ -18,34 +13,12 @@ export type SavedPreset = { createdAt: string; }; -export function readStorage(): SavedPreset[] { - if (typeof window === "undefined") return []; - try { - const raw = window.localStorage.getItem(STORAGE_KEY); - if (!raw) return []; - const parsed = JSON.parse(raw) as unknown; - if (!Array.isArray(parsed)) return []; - return parsed.filter(isSavedPreset); - } catch { - return []; - } -} - -function writeStorage(presets: SavedPreset[]) { - if (typeof window === "undefined") return; - try { - window.localStorage.setItem(STORAGE_KEY, JSON.stringify(presets)); - } catch { - /* ignore quota / access errors */ - } -} - const listeners = new Set<() => void>(); -let snapshot: SavedPreset[] | null = null; const emptySnapshot: SavedPreset[] = []; +let snapshot: SavedPreset[] = []; +let snapshotVersion = 0; function getSnapshot(): SavedPreset[] { - if (snapshot === null) snapshot = readStorage(); return snapshot; } @@ -62,7 +35,7 @@ function subscribe(cb: () => void) { function publish(next: SavedPreset[]) { snapshot = next; - writeStorage(next); + snapshotVersion += 1; for (const l of listeners) l(); } @@ -84,33 +57,14 @@ function readServerPresets(value: unknown): SavedPreset[] | null { return value.filter(isSavedPreset); } -const syncServer = createQueuedUserSettingsSync(SYNC_FAILED_KEY, (next) => ({ +const syncServer = createQueuedUserSettingsSync((next) => ({ gitlab_saved_presets: next, })); -function snapshotKey(value: SavedPreset[]): string { - return JSON.stringify(value); -} - -function hasMigratedToBackend(): boolean { - if (typeof window === "undefined") return false; - return window.localStorage.getItem(MIGRATED_KEY) === "1"; -} - -function markMigratedToBackend(): void { - if (typeof window === "undefined") return; - try { - window.localStorage.setItem(MIGRATED_KEY, "1"); - } catch { - /* ignore storage failures */ - } -} - -// Test-only: drop the module-level snapshot so the next read goes through -// readStorage again. Used by the hook tests so each `it` starts from a known -// empty state independent of test execution order. +// Test-only: isolate the module-level snapshot between hook tests. export function __resetSnapshotForTests() { - snapshot = null; + snapshot = []; + snapshotVersion = 0; for (const l of listeners) l(); } @@ -119,24 +73,12 @@ export function useSavedPresets() { useEffect(() => { let cancelled = false; - const initialKey = snapshotKey(readStorage()); + const initialVersion = snapshotVersion; fetchUserSettings({ cache: "no-store" }) .then((response) => { const serverPresets = readServerPresets(response.settings.gitlab_saved_presets); - if (cancelled || !serverPresets) return; - const local = readStorage(); - if (snapshotKey(local) !== initialKey) return; - if (hasUserSettingsSyncFailure(SYNC_FAILED_KEY)) { - void syncServer(local); - return; - } - if (serverPresets.length === 0 && local.length > 0 && !hasMigratedToBackend()) { - void syncServer(local); - markMigratedToBackend(); - return; - } + if (cancelled || !serverPresets || snapshotVersion !== initialVersion) return; publish(serverPresets); - markMigratedToBackend(); }) .catch(() => {}); return () => { @@ -144,28 +86,22 @@ export function useSavedPresets() { }; }, []); - // Always merge against the latest localStorage read instead of the in-memory - // snapshot. With two tabs open, snapshot in tab A is stale the moment tab B - // writes — appending to it would silently drop B's preset. readStorage is - // cheap (small JSON, single key) so this is fine on the save/remove paths. const save = useCallback((input: Omit) => { const preset: SavedPreset = { ...input, id: `g_${Date.now().toString(36)}_${Math.random().toString(36).slice(2, 8)}`, createdAt: new Date().toISOString(), }; - const next = [...readStorage(), preset]; + const next = [...getSnapshot(), preset]; publish(next); void syncServer(next); - markMigratedToBackend(); return preset; }, []); const remove = useCallback((id: string) => { - const next = readStorage().filter((p) => p.id !== id); + const next = getSnapshot().filter((p) => p.id !== id); publish(next); void syncServer(next); - markMigratedToBackend(); }, []); return { presets, save, remove }; diff --git a/apps/web/components/jira/my-jira/presets.ts b/apps/web/components/jira/my-jira/presets.ts index 6b01b76b7f..09d0de374f 100644 --- a/apps/web/components/jira/my-jira/presets.ts +++ b/apps/web/components/jira/my-jira/presets.ts @@ -31,7 +31,7 @@ export type JiraPresetIcon = | "sparkle" | "check"; -// Persisted shape — stored in localStorage and edited from settings. +// Persisted shape stored in backend user settings and edited from settings. export type JiraStoredPreset = { id: string; label: string; diff --git a/apps/web/components/jira/my-jira/use-saved-views.test.ts b/apps/web/components/jira/my-jira/use-saved-views.test.ts index 380c31eb11..13792d30f3 100644 --- a/apps/web/components/jira/my-jira/use-saved-views.test.ts +++ b/apps/web/components/jira/my-jira/use-saved-views.test.ts @@ -1,10 +1,9 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; -import { renderHook, waitFor } from "@testing-library/react"; +import { act, renderHook, waitFor } from "@testing-library/react"; import { fetchUserSettings, updateUserSettings } from "@/lib/api/domains/settings-api"; import { useSavedViews, type SavedView } from "./use-saved-views"; const STORAGE_KEY = "kandev:jira:saved-views:v1"; -const SYNC_FAILED_KEY = "kandev:jira:saved-views:sync-failed:v1"; vi.mock("@/lib/api/domains/settings-api", () => ({ fetchUserSettings: vi.fn(), @@ -25,6 +24,14 @@ function makeLocalStorageMock() { }; } +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise((res) => { + resolve = res; + }); + return { promise, resolve }; +} + const localStorageMock = makeLocalStorageMock(); vi.stubGlobal("localStorage", localStorageMock); @@ -53,16 +60,13 @@ describe("useSavedViews", () => { } as Awaited>); }); - it("retries local views after a failed backend sync and clears the marker", async () => { + it("ignores stale local views when backend settings are empty", async () => { localStorageMock.setItem(STORAGE_KEY, JSON.stringify([view])); - localStorageMock.setItem(SYNC_FAILED_KEY, "1"); - renderHook(() => useSavedViews()); + const { result } = renderHook(() => useSavedViews()); - await waitFor(() => { - expect(updateUserSettings).toHaveBeenCalledWith({ jira_saved_views: [view] }); - expect(localStorageMock.getItem(SYNC_FAILED_KEY)).toBeNull(); - }); + await waitFor(() => expect(result.current.custom).toEqual([])); + expect(updateUserSettings).not.toHaveBeenCalled(); }); it("hydrates a legacy statusCategories view to statuses: [] without throwing", async () => { @@ -79,7 +83,9 @@ describe("useSavedViews", () => { sort: "updated", }, }; - localStorageMock.setItem(STORAGE_KEY, JSON.stringify([legacy])); + vi.mocked(fetchUserSettings).mockResolvedValue({ + settings: { jira_saved_views: [legacy] }, + } as Awaited>); const { result } = renderHook(() => useSavedViews()); @@ -92,18 +98,68 @@ describe("useSavedViews", () => { }); }); - it("retries empty local views after a failed backend sync", async () => { - localStorageMock.setItem(STORAGE_KEY, JSON.stringify([])); - localStorageMock.setItem(SYNC_FAILED_KEY, "1"); - vi.mocked(fetchUserSettings).mockResolvedValue({ - settings: { jira_saved_views: [view] }, + it("replays a saved view mutation on hydrated server views", async () => { + const settings = deferred>>(); + vi.mocked(fetchUserSettings).mockReturnValueOnce(settings.promise); + const serverView = { ...view, id: "custom:server", name: "Server view" }; + + const { result } = renderHook(() => useSavedViews()); + const saved = result.current.save("New view", view.filters, null); + + expect(updateUserSettings).not.toHaveBeenCalled(); + + settings.resolve({ + settings: { jira_saved_views: [serverView] }, } as Awaited>); - renderHook(() => useSavedViews()); - await waitFor(() => { - expect(updateUserSettings).toHaveBeenCalledWith({ jira_saved_views: [] }); - expect(localStorageMock.getItem(SYNC_FAILED_KEY)).toBeNull(); + expect(result.current.custom.map((savedView) => savedView.id)).toEqual([ + "custom:server", + saved.id, + ]); + expect(updateUserSettings).toHaveBeenCalledWith({ + jira_saved_views: expect.arrayContaining([ + expect.objectContaining({ id: "custom:server" }), + expect.objectContaining({ id: saved.id }), + ]), + }); + }); + }); + + it("does not sync queued mutations when fetching settings fails", async () => { + vi.mocked(fetchUserSettings).mockRejectedValueOnce(new Error("network unavailable")); + vi.mocked(fetchUserSettings).mockRejectedValueOnce(new Error("network still unavailable")); + const fetchCallsBefore = vi.mocked(fetchUserSettings).mock.calls.length; + const syncCallsBefore = vi.mocked(updateUserSettings).mock.calls.length; + + const { result } = renderHook(() => useSavedViews()); + await waitFor(() => expect(fetchUserSettings).toHaveBeenCalledTimes(fetchCallsBefore + 1)); + act(() => { + result.current.save("New view", view.filters, null); + }); + + await waitFor(() => expect(fetchUserSettings).toHaveBeenCalledTimes(fetchCallsBefore + 2)); + expect(updateUserSettings).toHaveBeenCalledTimes(syncCallsBefore); + }); + + it("keeps saved views visible and retries hydration after a fetch failure", async () => { + vi.mocked(fetchUserSettings).mockRejectedValueOnce(new Error("network unavailable")); + const fetchCallsBefore = vi.mocked(fetchUserSettings).mock.calls.length; + + const { result } = renderHook(() => useSavedViews()); + await waitFor(() => expect(fetchUserSettings).toHaveBeenCalledTimes(fetchCallsBefore + 1)); + + let saved!: SavedView; + act(() => { + saved = result.current.save("New view", view.filters, null); + }); + + expect(result.current.custom.map((savedView) => savedView.id)).toEqual([saved.id]); + await waitFor(() => { + expect(fetchUserSettings).toHaveBeenCalledTimes(fetchCallsBefore + 2); + expect(updateUserSettings).toHaveBeenCalledWith({ + jira_saved_views: [expect.objectContaining({ id: saved.id })], + }); }); }); }); diff --git a/apps/web/components/jira/my-jira/use-saved-views.ts b/apps/web/components/jira/my-jira/use-saved-views.ts index d59c4e25ae..eef09c39a3 100644 --- a/apps/web/components/jira/my-jira/use-saved-views.ts +++ b/apps/web/components/jira/my-jira/use-saved-views.ts @@ -1,10 +1,9 @@ "use client"; -import { useCallback, useEffect, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { DEFAULT_FILTERS, type FilterState } from "./filter-model"; import { fetchUserSettings } from "@/lib/api/domains/settings-api"; import { createQueuedUserSettingsSync } from "@/lib/user-settings-sync"; -import { hasUserSettingsSyncFailure } from "@/lib/user-settings-sync-failure"; export type SavedView = { id: string; @@ -17,10 +16,6 @@ export type SavedView = { builtin?: boolean; }; -const STORAGE_KEY = "kandev:jira:saved-views:v1"; -const MIGRATED_KEY = "kandev:jira:saved-views:migrated-to-backend:v1"; -const SYNC_FAILED_KEY = "kandev:jira:saved-views:sync-failed:v1"; - const BUILTIN_VIEWS: SavedView[] = [ { id: "builtin:assigned", @@ -41,19 +36,6 @@ const BUILTIN_VIEWS: SavedView[] = [ }, ]; -function readStorage(): SavedView[] { - if (typeof window === "undefined") return []; - try { - const raw = window.localStorage.getItem(STORAGE_KEY); - if (!raw) return []; - const parsed = JSON.parse(raw) as unknown; - if (!Array.isArray(parsed)) return []; - return normalizeSavedViews(parsed); - } catch { - return []; - } -} - // isFilterStateShape recognizes both the current `statuses: string[]` shape and // legacy views persisted with `statusCategories`. It only validates the fields // unrelated to statuses; status normalization happens in normalizeFilterState. @@ -105,89 +87,85 @@ function normalizeSavedViews(values: unknown[]): SavedView[] { return values.map(normalizeSavedView).filter((v): v is SavedView => v !== null); } -function writeStorage(views: SavedView[]): void { - if (typeof window === "undefined") return; - try { - window.localStorage.setItem(STORAGE_KEY, JSON.stringify(views)); - } catch { - // Quota or private-mode: swallow. Views just won't persist. - } -} - function readServerViews(value: unknown): SavedView[] | null { if (!Array.isArray(value)) return null; return normalizeSavedViews(value); } -const syncServer = createQueuedUserSettingsSync(SYNC_FAILED_KEY, (views) => ({ +const syncServer = createQueuedUserSettingsSync((views) => ({ jira_saved_views: views, })); -function snapshotKey(views: SavedView[]): string { - return JSON.stringify(views); -} - -function hasMigratedToBackend(): boolean { - if (typeof window === "undefined") return false; - return window.localStorage.getItem(MIGRATED_KEY) === "1"; -} - -function markMigratedToBackend(): void { - if (typeof window === "undefined") return; - try { - window.localStorage.setItem(MIGRATED_KEY, "1"); - } catch { - // Ignore write failures. - } -} - -function latestSavedViews(fallback: SavedView[]): SavedView[] { - if (typeof window === "undefined") return fallback; - try { - const raw = window.localStorage.getItem(STORAGE_KEY); - if (raw === null) return fallback; - const parsed = JSON.parse(raw) as unknown; - if (!Array.isArray(parsed)) return fallback; - return normalizeSavedViews(parsed); - } catch { - return fallback; - } -} +type PendingMutation = (views: SavedView[]) => SavedView[]; export function useSavedViews() { const [custom, setCustom] = useState([]); + const customRef = useRef(custom); + const hydrated = useRef(false); + const mounted = useRef(true); + const hydrationRequest = useRef | null>(null); + const pendingMutations = useRef([]); - useEffect(() => { - let cancelled = false; - async function init() { - const loaded = readStorage(); - const initialKey = snapshotKey(loaded); - if (!cancelled) setCustom(loaded); - const response = await fetchUserSettings({ cache: "no-store" }).catch(() => null); - const serverViews = readServerViews(response?.settings.jira_saved_views); - if (!cancelled && serverViews) { - const local = readStorage(); - if (snapshotKey(local) !== initialKey) return; - if (hasUserSettingsSyncFailure(SYNC_FAILED_KEY)) { - void syncServer(local); - return; - } - if (serverViews.length === 0 && local.length > 0 && !hasMigratedToBackend()) { - void syncServer(local); - markMigratedToBackend(); - return; - } - writeStorage(serverViews); - setCustom(serverViews); - markMigratedToBackend(); + const hydrate = useCallback(async () => { + const activeRequest = hydrationRequest.current; + if (activeRequest) { + await activeRequest; + if (hydrated.current || !mounted.current) return; + if (hydrationRequest.current) { + await hydrationRequest.current; + return; } } - void init(); - return () => { - cancelled = true; - }; + + const request = (async () => { + const response = await fetchUserSettings({ cache: "no-store" }).catch(() => null); + if (!response || !mounted.current) return; + const serverViews = readServerViews(response.settings.jira_saved_views); + + const mutations = pendingMutations.current; + pendingMutations.current = []; + const hydratedViews = mutations.reduce((views, mutate) => mutate(views), serverViews ?? []); + hydrated.current = true; + customRef.current = hydratedViews; + setCustom(hydratedViews); + if (mutations.length > 0) { + void syncServer(hydratedViews); + } + })(); + hydrationRequest.current = request; + try { + await request; + } finally { + hydrationRequest.current = null; + } }, []); + useEffect(() => { + mounted.current = true; + void hydrate(); + return () => { + mounted.current = false; + }; + }, [hydrate]); + + const commitMutation = useCallback((mutate: PendingMutation) => { + const next = mutate(customRef.current); + customRef.current = next; + setCustom(next); + void syncServer(next); + }, []); + + const stageMutation = useCallback( + (mutate: PendingMutation) => { + pendingMutations.current.push(mutate); + const next = mutate(customRef.current); + customRef.current = next; + setCustom(next); + void hydrate(); + }, + [hydrate], + ); + const save = useCallback( (name: string, filters: FilterState, customJql: string | null): SavedView => { const view: SavedView = { @@ -196,27 +174,28 @@ export function useSavedViews() { filters, customJql, }; - setCustom((prev) => { - const next = [...latestSavedViews(prev), view]; - writeStorage(next); - void syncServer(next); - markMigratedToBackend(); - return next; - }); + const mutate: PendingMutation = (views) => [...views, view]; + if (!hydrated.current) { + stageMutation(mutate); + return view; + } + commitMutation(mutate); return view; }, - [], + [commitMutation, stageMutation], ); - const remove = useCallback((id: string) => { - setCustom((prev) => { - const next = latestSavedViews(prev).filter((v) => v.id !== id); - writeStorage(next); - void syncServer(next); - markMigratedToBackend(); - return next; - }); - }, []); + const remove = useCallback( + (id: string) => { + const mutate: PendingMutation = (views) => views.filter((v) => v.id !== id); + if (!hydrated.current) { + stageMutation(mutate); + return; + } + commitMutation(mutate); + }, + [commitMutation, stageMutation], + ); return { views: [...BUILTIN_VIEWS, ...custom], diff --git a/apps/web/components/jira/my-jira/use-task-presets.test.ts b/apps/web/components/jira/my-jira/use-task-presets.test.ts index 652491d9bb..c66c3b5d57 100644 --- a/apps/web/components/jira/my-jira/use-task-presets.test.ts +++ b/apps/web/components/jira/my-jira/use-task-presets.test.ts @@ -5,7 +5,6 @@ import { useJiraTaskPresets } from "./use-task-presets"; import type { JiraStoredPreset } from "./presets"; const STORAGE_KEY = "kandev:jira:task-presets:v1"; -const SYNC_FAILED_KEY = "kandev:jira:task-presets:sync-failed:v1"; vi.mock("@/lib/api/domains/settings-api", () => ({ fetchUserSettings: vi.fn(), @@ -48,29 +47,12 @@ describe("useJiraTaskPresets", () => { } as Awaited>); }); - it("retries local presets after a failed backend sync and clears the marker", async () => { + it("ignores stale local presets when backend settings are empty", async () => { localStorageMock.setItem(STORAGE_KEY, JSON.stringify([preset])); - localStorageMock.setItem(SYNC_FAILED_KEY, "1"); - renderHook(() => useJiraTaskPresets()); + const { result } = renderHook(() => useJiraTaskPresets()); - await waitFor(() => { - expect(updateUserSettings).toHaveBeenCalledWith({ jira_task_presets: [preset] }); - expect(localStorageMock.getItem(SYNC_FAILED_KEY)).toBeNull(); - }); - }); - - it("retries null local presets after a failed reset sync", async () => { - localStorageMock.setItem(SYNC_FAILED_KEY, "1"); - vi.mocked(fetchUserSettings).mockResolvedValue({ - settings: { jira_task_presets: [preset] }, - } as Awaited>); - - renderHook(() => useJiraTaskPresets()); - - await waitFor(() => { - expect(updateUserSettings).toHaveBeenCalledWith({ jira_task_presets: null }); - expect(localStorageMock.getItem(SYNC_FAILED_KEY)).toBeNull(); - }); + await waitFor(() => expect(result.current.stored).not.toContainEqual(preset)); + expect(updateUserSettings).not.toHaveBeenCalled(); }); }); diff --git a/apps/web/components/jira/my-jira/use-task-presets.ts b/apps/web/components/jira/my-jira/use-task-presets.ts index 6a99a6e5e2..a3a8de39d4 100644 --- a/apps/web/components/jira/my-jira/use-task-presets.ts +++ b/apps/web/components/jira/my-jira/use-task-presets.ts @@ -1,6 +1,6 @@ "use client"; -import { useCallback, useEffect, useMemo, useState } from "react"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { DEFAULT_JIRA_PRESETS, resolveJiraTaskPresets, @@ -9,11 +9,6 @@ import { } from "./presets"; import { fetchUserSettings } from "@/lib/api/domains/settings-api"; import { createQueuedUserSettingsSync } from "@/lib/user-settings-sync"; -import { hasUserSettingsSyncFailure } from "@/lib/user-settings-sync-failure"; - -const STORAGE_KEY = "kandev:jira:task-presets:v1"; -const MIGRATED_KEY = "kandev:jira:task-presets:migrated-to-backend:v1"; -const SYNC_FAILED_KEY = "kandev:jira:task-presets:sync-failed:v1"; function isStoredPreset(v: unknown): v is JiraStoredPreset { if (!v || typeof v !== "object") return false; @@ -27,95 +22,31 @@ function isStoredPreset(v: unknown): v is JiraStoredPreset { ); } -function readStorage(): JiraStoredPreset[] | null { - if (typeof window === "undefined") return null; - try { - const raw = window.localStorage.getItem(STORAGE_KEY); - if (!raw) return null; - const parsed = JSON.parse(raw) as unknown; - if (!Array.isArray(parsed)) return null; - // An explicitly-saved empty array means "the user cleared their presets" - // and should beat the built-in defaults. Only the absent-key case should - // fall back to defaults. - return parsed.filter(isStoredPreset); - } catch { - return null; - } -} - -function writeStorage(presets: JiraStoredPreset[] | null): void { - if (typeof window === "undefined") return; - try { - if (presets === null) { - window.localStorage.removeItem(STORAGE_KEY); - } else { - window.localStorage.setItem(STORAGE_KEY, JSON.stringify(presets)); - } - } catch { - // Quota or private-mode: swallow. Presets just won't persist. - } -} - function readServerPresets(value: unknown): JiraStoredPreset[] | null | undefined { if (value === null) return null; if (!Array.isArray(value)) return undefined; return value.filter(isStoredPreset); } -const syncServer = createQueuedUserSettingsSync( - SYNC_FAILED_KEY, - (presets) => ({ jira_task_presets: presets }), -); - -function snapshotKey(presets: JiraStoredPreset[] | null): string { - return JSON.stringify(presets); -} - -function hasMigratedToBackend(): boolean { - if (typeof window === "undefined") return false; - return window.localStorage.getItem(MIGRATED_KEY) === "1"; -} - -function markMigratedToBackend(): void { - if (typeof window === "undefined") return; - try { - window.localStorage.setItem(MIGRATED_KEY, "1"); - } catch { - // Ignore write failures. - } -} +const syncServer = createQueuedUserSettingsSync((presets) => ({ + jira_task_presets: presets, +})); export function useJiraTaskPresets() { const [stored, setStored] = useState(null); const [loaded, setLoaded] = useState(false); + const writeVersion = useRef(0); useEffect(() => { let cancelled = false; + const initialVersion = writeVersion.current; async function init() { - const value = readStorage(); - const initialKey = snapshotKey(value); - if (!cancelled) { - setStored(value); - setLoaded(true); - } const response = await fetchUserSettings({ cache: "no-store" }).catch(() => null); const serverValue = readServerPresets(response?.settings.jira_task_presets); - if (!cancelled && serverValue !== undefined) { - const local = readStorage(); - if (snapshotKey(local) !== initialKey) return; - if (hasUserSettingsSyncFailure(SYNC_FAILED_KEY)) { - void syncServer(local); - return; - } - if (serverValue === null && local !== null && !hasMigratedToBackend()) { - void syncServer(local); - markMigratedToBackend(); - return; - } - writeStorage(serverValue); + if (!cancelled && serverValue !== undefined && writeVersion.current === initialVersion) { setStored(serverValue); - markMigratedToBackend(); } + if (!cancelled) setLoaded(true); } void init(); return () => { @@ -124,16 +55,14 @@ export function useJiraTaskPresets() { }, []); const save = useCallback((next: JiraStoredPreset[]) => { - writeStorage(next); + writeVersion.current += 1; void syncServer(next); - markMigratedToBackend(); setStored(next); }, []); const reset = useCallback(() => { - writeStorage(null); + writeVersion.current += 1; void syncServer(null); - markMigratedToBackend(); setStored(null); }, []); diff --git a/apps/web/components/jira/task-presets-section.tsx b/apps/web/components/jira/task-presets-section.tsx index 7ae169fd2a..2bbb2d42fc 100644 --- a/apps/web/components/jira/task-presets-section.tsx +++ b/apps/web/components/jira/task-presets-section.tsx @@ -169,8 +169,8 @@ function usePresetDraft() { const [draft, setDraft] = useState(stored); // Render-time conditional setState is React's documented "adjust state // during render" pattern; it resets the draft when the hook's stored value - // changes (e.g. after reset or cross-tab edit). Gate the sync on `loaded` so - // an in-progress edit isn't wiped when the initial localStorage read lands. + // changes (e.g. after reset or a backend refresh). Gate the sync on `loaded` + // so an in-progress edit isn't wiped when the initial settings read lands. const [synced, setSynced] = useState(stored); if (loaded && stored !== synced) { setSynced(stored); diff --git a/apps/web/components/state-provider.test.tsx b/apps/web/components/state-provider.test.tsx index b7296974de..c8d3e4743d 100644 --- a/apps/web/components/state-provider.test.tsx +++ b/apps/web/components/state-provider.test.tsx @@ -1,8 +1,6 @@ -import { fireEvent, render, screen, waitFor } from "@testing-library/react"; -import { useEffect } from "react"; +import { fireEvent, render, screen } from "@testing-library/react"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { defaultSettingsState } from "@/lib/state/slices/settings/settings-slice"; -import { STORAGE_KEYS } from "@/lib/settings/constants"; import { readQueuedTaskCreateLastUsedState, resetTaskCreateLastUsedSync, @@ -38,54 +36,6 @@ function EnableMetricsFromNestedProvider() { ); } -function ObserveLastUsedCache({ onSeen }: { onSeen: (value: string | null) => void }) { - useEffect(() => { - onSeen(window.localStorage.getItem(STORAGE_KEYS.LAST_REPOSITORY_ID)); - }, [onSeen]); - return null; -} - -const cachedTaskCreateChoices = { - repositoryId: "repo-cached", - branch: "feature-cached", - agentProfileId: "agent-cached", - executorProfileId: "exec-cached", -}; - -function seedCachedTaskCreateChoices() { - window.localStorage.setItem( - STORAGE_KEYS.LAST_REPOSITORY_ID, - JSON.stringify(cachedTaskCreateChoices.repositoryId), - ); - window.localStorage.setItem( - STORAGE_KEYS.LAST_BRANCH, - JSON.stringify(cachedTaskCreateChoices.branch), - ); - window.localStorage.setItem( - STORAGE_KEYS.LAST_AGENT_PROFILE_ID, - JSON.stringify(cachedTaskCreateChoices.agentProfileId), - ); - window.localStorage.setItem( - STORAGE_KEYS.LAST_EXECUTOR_PROFILE_ID, - JSON.stringify(cachedTaskCreateChoices.executorProfileId), - ); -} - -function expectCachedTaskCreateChoices() { - expect(window.localStorage.getItem(STORAGE_KEYS.LAST_REPOSITORY_ID)).toBe( - JSON.stringify(cachedTaskCreateChoices.repositoryId), - ); - expect(window.localStorage.getItem(STORAGE_KEYS.LAST_BRANCH)).toBe( - JSON.stringify(cachedTaskCreateChoices.branch), - ); - expect(window.localStorage.getItem(STORAGE_KEYS.LAST_AGENT_PROFILE_ID)).toBe( - JSON.stringify(cachedTaskCreateChoices.agentProfileId), - ); - expect(window.localStorage.getItem(STORAGE_KEYS.LAST_EXECUTOR_PROFILE_ID)).toBe( - JSON.stringify(cachedTaskCreateChoices.executorProfileId), - ); -} - beforeEach(() => { window.localStorage.clear(); resetTaskCreateLastUsedSync({ clearQueued: true }); @@ -117,184 +67,6 @@ describe("StateProvider", () => { }); }); -describe("StateProvider task-create cache", () => { - it("syncs task-create last-used settings to localStorage without redundant writes", async () => { - function UpdateUnrelatedSetting() { - const setUserSettings = useAppStore((state) => state.setUserSettings); - const userSettings = useAppStore((state) => state.userSettings); - return ( - - ); - } - - const setItemSpy = vi.spyOn(Storage.prototype, "setItem"); - render( - - - , - ); - - await waitFor(() => { - expect(window.localStorage.getItem(STORAGE_KEYS.LAST_REPOSITORY_ID)).toBe( - JSON.stringify("repo-1"), - ); - }); - const writesAfterInitialSync = setItemSpy.mock.calls.length; - - fireEvent.click(screen.getByRole("button", { name: "Toggle unrelated" })); - - await waitFor(() => { - expect(screen.getByRole("button", { name: "Toggle unrelated" })).toBeTruthy(); - }); - expect(setItemSpy).toHaveBeenCalledTimes(writesAfterInitialSync); - }); - - it("primes task-create last-used localStorage before child effects run", () => { - const onSeen = vi.fn(); - render( - - - , - ); - - expect(onSeen).toHaveBeenCalledWith(JSON.stringify("repo-1")); - }); -}); - -describe("StateProvider task-create cache fallback", () => { - it("keeps cached task-create choices when loaded backend settings omit them", async () => { - const onSeen = vi.fn(); - seedCachedTaskCreateChoices(); - - render( - - - , - ); - - expect(onSeen).toHaveBeenCalledWith(JSON.stringify(cachedTaskCreateChoices.repositoryId)); - // Give any potential deferred deletions a chance to fire (old code used setTimeout(0)). - await new Promise((resolve) => window.setTimeout(resolve, 10)); - expectCachedTaskCreateChoices(); - }); - - it("keeps cached task-create choices when loaded backend settings sync an empty object", async () => { - seedCachedTaskCreateChoices(); - - render( - -
ready
- , - ); - - await waitFor(() => { - expectCachedTaskCreateChoices(); - }); - }); - - it("clears stale cached fields when loaded backend settings contain real task-create choices", async () => { - window.localStorage.setItem( - STORAGE_KEYS.LAST_BRANCH, - JSON.stringify(cachedTaskCreateChoices.branch), - ); - window.localStorage.setItem( - STORAGE_KEYS.LAST_AGENT_PROFILE_ID, - JSON.stringify(cachedTaskCreateChoices.agentProfileId), - ); - - render( - -
ready
- , - ); - - await waitFor(() => { - expect(window.localStorage.getItem(STORAGE_KEYS.LAST_REPOSITORY_ID)).toBe( - JSON.stringify("repo-server"), - ); - expect(window.localStorage.getItem(STORAGE_KEYS.LAST_BRANCH)).toBeNull(); - expect(window.localStorage.getItem(STORAGE_KEYS.LAST_AGENT_PROFILE_ID)).toBeNull(); - }); - }); -}); - describe("StateProvider task-create queued overlay", () => { it("clears the queued overlay when loaded settings catch up", () => { syncTaskCreateLastUsed({ repository_id: "repo-1", branch: "main" }); diff --git a/apps/web/components/state-provider.tsx b/apps/web/components/state-provider.tsx index 47088eb4ac..2de5d7ee58 100644 --- a/apps/web/components/state-provider.tsx +++ b/apps/web/components/state-provider.tsx @@ -14,8 +14,6 @@ import { useStore } from "zustand"; import { isDebug, registerSessionTaskResolver } from "@/lib/debug/log"; import type { AppState, StoreProviderProps } from "@/lib/state/store"; import { createAppStore } from "@/lib/state/store"; -import { removeLocalStorage, setLocalStorage } from "@/lib/local-storage"; -import { STORAGE_KEYS } from "@/lib/settings/constants"; import { clearQueuedTaskCreateLastUsedIfSynced } from "./task-create-dialog-handlers"; const StoreContext = createContext | null>(null); @@ -43,7 +41,7 @@ export function StateProvider({ children, initialState }: StoreProviderProps) { }, [store]); useLayoutEffect(() => { - syncTaskCreateLastUsedCache(store.getState()); + clearQueuedTaskCreateLastUsed(store.getState()); return store.subscribe((state, prevState) => { if ( state.userSettings.loaded === prevState.userSettings.loaded && @@ -54,7 +52,7 @@ export function StateProvider({ children, initialState }: StoreProviderProps) { ) { return; } - syncTaskCreateLastUsedCache(state); + clearQueuedTaskCreateLastUsed(state); }); }, [store]); @@ -71,38 +69,9 @@ export function StateProvider({ children, initialState }: StoreProviderProps) { return {children}; } -function syncTaskCreateLastUsedCache(state: AppState) { +function clearQueuedTaskCreateLastUsed(state: AppState) { if (!state.userSettings.loaded) return; - const lastUsed = state.userSettings.taskCreateLastUsed; - syncTaskCreateLastUsedCacheField( - STORAGE_KEYS.LAST_REPOSITORY_ID, - lastUsed?.repositoryId, - lastUsed?.synced, - ); - syncTaskCreateLastUsedCacheField(STORAGE_KEYS.LAST_BRANCH, lastUsed?.branch, lastUsed?.synced); - syncTaskCreateLastUsedCacheField( - STORAGE_KEYS.LAST_AGENT_PROFILE_ID, - lastUsed?.agentProfileId, - lastUsed?.synced, - ); - syncTaskCreateLastUsedCacheField( - STORAGE_KEYS.LAST_EXECUTOR_PROFILE_ID, - lastUsed?.executorProfileId, - lastUsed?.synced, - ); - clearQueuedTaskCreateLastUsedIfSynced(lastUsed); -} - -function syncTaskCreateLastUsedCacheField( - key: string, - value: string | null | undefined, - synced: boolean | undefined, -) { - if (value) { - setLocalStorage(key, value); - return; - } - if (synced) removeLocalStorage(key); + clearQueuedTaskCreateLastUsedIfSynced(state.userSettings.taskCreateLastUsed); } function taskCreateLastUsedEqual( diff --git a/apps/web/components/task-create-dialog-autopick.ts b/apps/web/components/task-create-dialog-autopick.ts index 2be9c5b705..b43b721f76 100644 --- a/apps/web/components/task-create-dialog-autopick.ts +++ b/apps/web/components/task-create-dialog-autopick.ts @@ -2,8 +2,6 @@ import { useEffect } from "react"; import type { AgentProfileOption } from "@/lib/state/slices"; -import { getLocalStorage } from "@/lib/local-storage"; -import { STORAGE_KEYS } from "@/lib/settings/constants"; import { createDebugLogger, isDebug } from "@/lib/debug/log"; import type { DialogFormState, StoreSelections } from "@/components/task-create-dialog-types"; @@ -23,7 +21,7 @@ const workflowAutopickDebug = createDebugLogger("executor-compat:workflow-autopi * effect can stay below the 100-line lint cap once the autopick trace logs * are inlined, and so the same decision can be tested without rendering. * - * Order: lastId (localStorage) → defId (workspace default) → first + * Order: last-used backend setting → workspace default → first * compatible. Every candidate is filtered against `compatibleAgentProfiles` * so a previously-used profile that's not wired for the chosen executor * isn't restored, then immediately fails the executor-compat gate. @@ -106,8 +104,8 @@ function buildAgentAutopickDebugFields(input: { current: input.agentProfileId || "-", workflow_id: input.selectedWorkflowId ?? "-", executor_profile_id: input.executorProfileId || "-", - local_storage_id: lastId ?? "-", - local_storage_valid: Boolean( + last_used_settings_id: lastId ?? "-", + last_used_settings_valid: Boolean( lastId && input.compatibleAgentProfiles.some((p) => p.id === lastId), ), workspace_default_id: defId ?? "-", @@ -125,14 +123,12 @@ function resolveLastUsedAgentProfileId( compatibleAgentProfiles: AgentProfileOption[], settingsAgentProfileId?: string | null, ) { - const localId = getLocalStorage(STORAGE_KEYS.LAST_AGENT_PROFILE_ID, null); if ( settingsAgentProfileId && compatibleAgentProfiles.some((p) => p.id === settingsAgentProfileId) ) { return settingsAgentProfileId; } - if (localId && compatibleAgentProfiles.some((p) => p.id === localId)) return localId; return null; } diff --git a/apps/web/components/task-create-dialog-effects-executor.test.ts b/apps/web/components/task-create-dialog-effects-executor.test.ts index 07c7d0b463..faf468e581 100644 --- a/apps/web/components/task-create-dialog-effects-executor.test.ts +++ b/apps/web/components/task-create-dialog-effects-executor.test.ts @@ -1,17 +1,15 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; +import { describe, it, expect, vi } from "vitest"; import { renderHook, waitFor } from "@testing-library/react"; import { useDefaultSelectionsEffect } from "./task-create-dialog-effects"; import type { DialogFormState, StoreSelections } from "@/components/task-create-dialog-types"; -import { STORAGE_KEYS } from "@/lib/settings/constants"; - -beforeEach(() => { - localStorage.clear(); -}); const PROFILE_DOCKER = "profile-docker"; const PROFILE_LOCAL = "profile-local"; const PROFILE_WORKTREE = "profile-worktree"; const PROFILE_WORKTREE_B = "profile-worktree-b"; +const EXECUTOR_DOCKER = "exec-docker"; +const REMOTE_URL_A = "github.com/acme/a"; +const REMOTE_URL_B = "github.com/acme/b"; type DefaultSelFake = Pick< DialogFormState, @@ -68,7 +66,7 @@ function localExecutor(): StoreSelections["executors"][number] { function dockerExecutor(): StoreSelections["executors"][number] { return { - id: "exec-docker", + id: EXECUTOR_DOCKER, type: "local_docker", profiles: [{ id: PROFILE_DOCKER, executor_type: "local_docker" }], } as unknown as StoreSelections["executors"][number]; @@ -168,7 +166,6 @@ describe("useDefaultSelectionsEffect - executor profile defaults", () => { describe("useDefaultSelectionsEffect - executor profile restoration", () => { it("defers executor profile fallback until user settings have loaded or settled", async () => { - window.localStorage.removeItem(STORAGE_KEYS.LAST_EXECUTOR_PROFILE_ID); const fs = makeDefaultSelFs({ executorProfileId: "", executorId: "" }); const worktree = worktreeExecutor(); const selBefore = makeSel({ @@ -192,46 +189,10 @@ describe("useDefaultSelectionsEffect - executor profile restoration", () => { await waitFor(() => expect(fs.setExecutorProfileId).toHaveBeenCalledWith(PROFILE_WORKTREE)); }); - - it("keeps deferring when a valid cached executor profile exists but settings are loading", async () => { - window.localStorage.setItem( - STORAGE_KEYS.LAST_EXECUTOR_PROFILE_ID, - JSON.stringify(PROFILE_WORKTREE), - ); - const fs = makeDefaultSelFs({ executorProfileId: "", executorId: "" }); - const sel = makeSel({ - executors: [worktreeExecutor()], - userSettingsLoaded: false, - }); - - renderHook(() => useDefaultSelectionsEffect(fs, true, sel, [])); - - await new Promise((resolve) => setTimeout(resolve, 10)); - expect(fs.setExecutorId).not.toHaveBeenCalled(); - expect(fs.setExecutorProfileId).not.toHaveBeenCalled(); - }); - - it("keeps deferring when cached executor profile is ineligible for the source mode", async () => { - window.localStorage.setItem( - STORAGE_KEYS.LAST_EXECUTOR_PROFILE_ID, - JSON.stringify(PROFILE_WORKTREE), - ); - const fs = makeDefaultSelFs({ executorProfileId: "", executorId: "", noRepository: true }); - const sel = makeSel({ - executors: [worktreeExecutor(), localExecutor()], - userSettingsLoaded: false, - }); - - renderHook(() => useDefaultSelectionsEffect(fs, true, sel, [])); - - await new Promise((resolve) => setTimeout(resolve, 10)); - expect(fs.setExecutorProfileId).not.toHaveBeenCalled(); - }); }); describe("useDefaultSelectionsEffect - executor profile settings restoration", () => { - it("restores executor profile from store-backed settings when localStorage is not primed", async () => { - window.localStorage.removeItem(STORAGE_KEYS.LAST_EXECUTOR_PROFILE_ID); + it("restores executor profile from backend settings", async () => { const fs = makeDefaultSelFs({ executorProfileId: "", executorId: "" }); const worktree = worktreeExecutor(); const sel = makeSel({ @@ -246,7 +207,6 @@ describe("useDefaultSelectionsEffect - executor profile settings restoration", ( }); it("does not pick a fallback executor id while a valid last-used profile is restoring", async () => { - window.localStorage.removeItem(STORAGE_KEYS.LAST_EXECUTOR_PROFILE_ID); const local = localExecutor(); const worktree = worktreeExecutor(); const sel = makeSel({ @@ -288,16 +248,36 @@ describe("useDefaultSelectionsEffect - executor profile settings restoration", ( }); describe("useDefaultSelectionsEffect - multi-repo guard counts Remote rows", () => { + it("prefers the saved worktree profile when multi-repo mode needs a switch", async () => { + const fs = makeDefaultSelFs({ + executorId: EXECUTOR_DOCKER, + executorProfileId: PROFILE_DOCKER, + useRemote: true, + remoteRepos: [ + { key: "remote-0", url: REMOTE_URL_A, branch: "", source: "paste" }, + { key: "remote-1", url: REMOTE_URL_B, branch: "", source: "paste" }, + ] as DialogFormState["remoteRepos"], + }); + const sel = makeSel({ + executors: [dockerExecutor(), worktreeExecutor()], + lastUsedExecutorProfileId: PROFILE_WORKTREE_B, + }); + + renderHook(() => useDefaultSelectionsEffect(fs, true, sel, [])); + + await waitFor(() => expect(fs.setExecutorProfileId).toHaveBeenCalledWith(PROFILE_WORKTREE_B)); + }); + it("swaps a non-worktree profile to worktree when 2+ Remote URL rows are set", async () => { // Regression: the guard used to count only workspace/local rows, so 2 // Remote rows slipped past it with an already-selected non-worktree profile. const fs = makeDefaultSelFs({ - executorId: "exec-docker", + executorId: EXECUTOR_DOCKER, executorProfileId: PROFILE_DOCKER, useRemote: true, remoteRepos: [ - { key: "remote-0", url: "github.com/acme/a", branch: "", source: "paste" }, - { key: "remote-1", url: "github.com/acme/b", branch: "", source: "paste" }, + { key: "remote-0", url: REMOTE_URL_A, branch: "", source: "paste" }, + { key: "remote-1", url: REMOTE_URL_B, branch: "", source: "paste" }, ] as DialogFormState["remoteRepos"], }); const sel = makeSel({ executors: [dockerExecutor(), worktreeExecutor()] }); @@ -313,8 +293,8 @@ describe("useDefaultSelectionsEffect - multi-repo guard counts Remote rows", () executorProfileId: PROFILE_WORKTREE, useRemote: true, remoteRepos: [ - { key: "remote-0", url: "github.com/acme/a", branch: "", source: "paste" }, - { key: "remote-1", url: "github.com/acme/b", branch: "", source: "paste" }, + { key: "remote-0", url: REMOTE_URL_A, branch: "", source: "paste" }, + { key: "remote-1", url: REMOTE_URL_B, branch: "", source: "paste" }, ] as DialogFormState["remoteRepos"], }); const sel = makeSel({ executors: [worktreeExecutor()] }); @@ -327,11 +307,11 @@ describe("useDefaultSelectionsEffect - multi-repo guard counts Remote rows", () it("does not swap when only a single Remote row is filled", async () => { const fs = makeDefaultSelFs({ - executorId: "exec-docker", + executorId: EXECUTOR_DOCKER, executorProfileId: PROFILE_DOCKER, useRemote: true, remoteRepos: [ - { key: "remote-0", url: "github.com/acme/a", branch: "", source: "paste" }, + { key: "remote-0", url: REMOTE_URL_A, branch: "", source: "paste" }, { key: "remote-1", url: "", branch: "", source: "paste" }, ] as DialogFormState["remoteRepos"], }); diff --git a/apps/web/components/task-create-dialog-effects.test.ts b/apps/web/components/task-create-dialog-effects.test.ts index 697d4edc31..1c377e3e2c 100644 --- a/apps/web/components/task-create-dialog-effects.test.ts +++ b/apps/web/components/task-create-dialog-effects.test.ts @@ -8,7 +8,7 @@ import { decideAgentProfileAutopick } from "./task-create-dialog-autopick"; import type { DialogFormState, StoreSelections } from "@/components/task-create-dialog-types"; import type { AgentProfileOption } from "@/lib/state/slices"; import type { Workspace } from "@/lib/types/http"; -import { STORAGE_KEYS } from "@/lib/settings/constants"; +const STORAGE_KEYS = { LAST_AGENT_PROFILE_ID: "kandev.dialog.lastAgentProfileId" } as const; // Minimal fake of DialogFormState - the hook destructures only three fields, // so the rest can be undefined behind an `as` cast and never read. @@ -49,7 +49,6 @@ beforeEach(() => { describe("useWorkflowAgentProfileEffect", () => { it("clears workflowAgentProfileId and leaves agentProfileId alone when no workflow is selected", () => { - localStorage.setItem(STORAGE_KEYS.LAST_AGENT_PROFILE_ID, JSON.stringify("stale-id")); const fs = makeFs({ selectedWorkflowId: null }); renderHook(() => @@ -98,8 +97,7 @@ describe("useWorkflowAgentProfileEffect", () => { expect(fs.setAgentProfileId).not.toHaveBeenCalledWith("missing-id"); }); - it("restores last-used agentProfileId when the workflow has no override and the id is still valid", () => { - localStorage.setItem(STORAGE_KEYS.LAST_AGENT_PROFILE_ID, JSON.stringify("real-id")); + it("restores backend last-used agentProfileId when the workflow has no override", () => { const fs = makeFs({ selectedWorkflowId: "wf-1" }); const workflows = [{ id: "wf-1" /* no agent_profile_id */ }]; @@ -109,6 +107,7 @@ describe("useWorkflowAgentProfileEffect", () => { workflows, [makeProfile("real-id")], [makeProfile("real-id")], + { lastUsedAgentProfileId: "real-id" }, ), ); @@ -315,9 +314,8 @@ describe("decideAgentProfileAutopick — user settings deferral", () => { expect(picked).toEqual({ kind: "pick", source: "first", id: cursor.id }); }); - it("defers auto-pick while user settings load even when localStorage has a valid profile", () => { + it("defers auto-pick while user settings load", () => { const cursor = makeProfile("cursor"); - localStorage.setItem(STORAGE_KEYS.LAST_AGENT_PROFILE_ID, JSON.stringify(cursor.id)); const deferred = decideAgentProfileAutopick({ open: true, @@ -342,7 +340,6 @@ describe("useDefaultSelectionsEffect — executor-aware agent restoration", () = it("restores lastId when it is compatible with the current executor", async () => { const claude = makeProfile("claude"); const cursor = makeProfile("cursor"); - localStorage.setItem(STORAGE_KEYS.LAST_AGENT_PROFILE_ID, JSON.stringify(cursor.id)); const fs = makeDefaultSelFs(); const sel = makeSel({ agentProfiles: [claude, cursor], @@ -364,7 +361,6 @@ describe("useDefaultSelectionsEffect — executor-aware agent restoration", () = const claude = makeProfile("claude"); const cursor = makeProfile("cursor"); const codex = makeProfile("codex"); - localStorage.setItem(STORAGE_KEYS.LAST_AGENT_PROFILE_ID, JSON.stringify(claude.id)); const fs = makeDefaultSelFs(); const sel = makeSel({ agentProfiles: [claude, cursor, codex], @@ -381,7 +377,6 @@ describe("useDefaultSelectionsEffect — executor-aware agent restoration", () = const claude = makeProfile("claude"); const cursor = makeProfile("cursor"); const codex = makeProfile("codex"); - localStorage.setItem(STORAGE_KEYS.LAST_AGENT_PROFILE_ID, JSON.stringify(claude.id)); const fs = makeDefaultSelFs(); const sel = makeSel({ agentProfiles: [claude, cursor, codex], @@ -399,7 +394,6 @@ describe("useDefaultSelectionsEffect — executor-aware agent restoration", () = const claude = makeProfile("claude"); const gemini = makeProfile("gemini"); const cursor = makeProfile("cursor"); - localStorage.setItem(STORAGE_KEYS.LAST_AGENT_PROFILE_ID, JSON.stringify(claude.id)); const fs = makeDefaultSelFs(); const sel = makeSel({ agentProfiles: [claude, gemini, cursor], @@ -423,7 +417,6 @@ describe("useDefaultSelectionsEffect — executor-aware agent restoration", () = // producing a worse UX (selector populated, but submit still blocked). const claude = makeProfile("claude"); const gemini = makeProfile("gemini"); - localStorage.setItem(STORAGE_KEYS.LAST_AGENT_PROFILE_ID, JSON.stringify(claude.id)); const fs = makeDefaultSelFs(); const sel = makeSel({ agentProfiles: [claude, gemini], @@ -440,7 +433,6 @@ describe("useDefaultSelectionsEffect — executor-aware agent restoration", () = it("does nothing when agentProfileId is already set (user has explicitly picked)", async () => { const claude = makeProfile("claude"); const cursor = makeProfile("cursor"); - localStorage.setItem(STORAGE_KEYS.LAST_AGENT_PROFILE_ID, JSON.stringify(cursor.id)); const fs = makeDefaultSelFs({ agentProfileId: claude.id }); const sel = makeSel({ agentProfiles: [claude, cursor], @@ -466,7 +458,6 @@ describe("useDefaultSelectionsEffect — auth-spec load race", () => { // later, the filter narrows the list, and `noCompatibleAgent` flips true. const claude = makeProfile("claude"); const cursor = makeProfile("cursor"); - localStorage.setItem(STORAGE_KEYS.LAST_AGENT_PROFILE_ID, JSON.stringify(claude.id)); const fs = makeDefaultSelFs({ executorProfileId: "" }); // Mirror the production shape: specs loaded, no executor picked yet, so // useExecutorProfileCompat returns agentProfiles unfiltered. @@ -508,7 +499,6 @@ describe("useDefaultSelectionsEffect — auth-spec load race", () => { const claude = makeProfile("claude"); const cursor = makeProfile("cursor"); const codex = makeProfile("codex"); - localStorage.setItem(STORAGE_KEYS.LAST_AGENT_PROFILE_ID, JSON.stringify(claude.id)); const fs = makeDefaultSelFs(); // Phase 1: specs not yet loaded — compatibleAgentProfiles is the full // list because the executor-compat hook returns agentProfiles when diff --git a/apps/web/components/task-create-dialog-effects.ts b/apps/web/components/task-create-dialog-effects.ts index 6e195c4776..7e8dc85eed 100644 --- a/apps/web/components/task-create-dialog-effects.ts +++ b/apps/web/components/task-create-dialog-effects.ts @@ -9,8 +9,6 @@ import { getLocalRepositoryStatusAction, } from "@/app/actions/workspaces"; import { listWorkflowSteps } from "@/lib/api/domains/workflow-api"; -import { getLocalStorage } from "@/lib/local-storage"; -import { STORAGE_KEYS } from "@/lib/settings/constants"; import { parseGitHubAnyUrl } from "@/hooks/domains/github/use-pr-info-by-url"; import type { DialogFormState, @@ -229,14 +227,12 @@ function pickDefaultExecutorProfileId( : allProfiles; if (eligibleProfiles.length === 0) return null; - const lastId = getLocalStorage(STORAGE_KEYS.LAST_EXECUTOR_PROFILE_ID, null); if ( lastUsedExecutorProfileId && eligibleProfiles.some((p) => p.id === lastUsedExecutorProfileId) ) { return lastUsedExecutorProfileId; } - if (lastId && eligibleProfiles.some((p) => p.id === lastId)) return lastId; const executorId = pickDefaultExecutorId( executors, @@ -260,8 +256,6 @@ type ExecutorAutopickContext = { type ExecutorProfileLastUsedState = { allProfiles: ExecutorProfileCandidate[]; eligibleProfiles: ExecutorProfileCandidate[]; - localStorageId: string | null; - localStorageValid: boolean; settingsId: string | null; settingsValid: boolean; }; @@ -276,17 +270,9 @@ function getExecutorProfileLastUsedState( noRepository || preferLocalExecutor ? allProfiles.filter((p) => !isWorktreeExecutorType(p._executorType)) : allProfiles; - const localStorageId = getLocalStorage( - STORAGE_KEYS.LAST_EXECUTOR_PROFILE_ID, - null, - ); return { allProfiles, eligibleProfiles, - localStorageId, - localStorageValid: Boolean( - localStorageId && eligibleProfiles.some((p) => p.id === localStorageId), - ), settingsId, settingsValid: Boolean(settingsId && eligibleProfiles.some((p) => p.id === settingsId)), }; @@ -304,7 +290,7 @@ export function shouldWaitForLastUsedExecutorProfile(context: ExecutorAutopickCo context, context.lastUsedExecutorProfileId ?? null, ); - if (!lastUsedProfile.localStorageValid && !lastUsedProfile.settingsValid) return false; + if (!lastUsedProfile.settingsValid) return false; if (isDebug()) { selectionDebug("executor-autopick", { current: "-", @@ -329,8 +315,6 @@ function logExecutorProfileAutopick( selectionDebug("executor-profile-autopick", { current: "-", pick: pick ?? "-", - local_storage_id: lastUsed.localStorageId ?? "-", - local_storage_valid: lastUsed.localStorageValid, settings_id: lastUsed.settingsId ?? "-", settings_valid: lastUsed.settingsValid, profile_count: lastUsed.allProfiles.length, @@ -415,7 +399,7 @@ function useExecutorProfileAutopickEffect({ }) { const { executors, workspaceDefaults, noRepository, preferLocalExecutor } = context; useEffect(() => { - // Auto-select executor profile: last used (localStorage) → source-aware + // Auto-select executor profile: last-used backend setting → source-aware // executor default → first eligible profile. if (!open || executorProfileId || executors.length === 0) return; const lastUsed = getExecutorProfileLastUsedState( @@ -527,13 +511,14 @@ export function useDefaultSelectionsEffect( } as DialogFormState), [noRepository, useRemote, remoteRepos, repositories], ); - useMultiRepoGuardEffect( + useMultiRepoGuardEffect({ open, executorProfileId, setExecutorProfileId, executors, selectedRepoCount, - ); + lastUsedExecutorProfileId: sel.lastUsedExecutorProfileId ?? null, + }); } /** diff --git a/apps/web/components/task-create-dialog-executor-wait.test.ts b/apps/web/components/task-create-dialog-executor-wait.test.ts index 43eb0628bf..5012227a09 100644 --- a/apps/web/components/task-create-dialog-executor-wait.test.ts +++ b/apps/web/components/task-create-dialog-executor-wait.test.ts @@ -1,6 +1,5 @@ -import { describe, expect, it, beforeEach } from "vitest"; +import { describe, expect, it } from "vitest"; import { shouldWaitForLastUsedExecutorProfile } from "./task-create-dialog-effects"; -import { STORAGE_KEYS } from "@/lib/settings/constants"; import type { StoreSelections } from "@/components/task-create-dialog-types"; const WORKTREE_EXECUTOR_ID = "exec-worktree"; @@ -14,10 +13,6 @@ function makeWorktreeExecutor(): StoreSelections["executors"][number] { } as unknown as StoreSelections["executors"][number]; } -beforeEach(() => { - window.localStorage.removeItem(STORAGE_KEYS.LAST_EXECUTOR_PROFILE_ID); -}); - describe("shouldWaitForLastUsedExecutorProfile", () => { it("waits only while a valid last-used executor profile can restore", () => { const worktreeExecutor = makeWorktreeExecutor(); diff --git a/apps/web/components/task-create-dialog-handlers.ts b/apps/web/components/task-create-dialog-handlers.ts index 772f7e3fce..7eaf9204da 100644 --- a/apps/web/components/task-create-dialog-handlers.ts +++ b/apps/web/components/task-create-dialog-handlers.ts @@ -3,8 +3,6 @@ import { useCallback } from "react"; import type { Repository } from "@/lib/types/http"; import type { DialogFormState, TaskRepoRow } from "@/components/task-create-dialog-types"; -import { removeLocalStorage, setLocalStorage } from "@/lib/local-storage"; -import { STORAGE_KEYS } from "@/lib/settings/constants"; import { createDebugLogger } from "@/lib/debug/log"; import type { TaskCreateLastUsedState } from "@/lib/state/slices/settings/types"; @@ -27,7 +25,6 @@ type TaskCreateLastUsedPayload = { }; let lastQueuedLastUsed: Partial = {}; -const LOCAL_STORAGE_WRITE_EVENT = "localStorage-write"; const lastUsedDebug = createDebugLogger("task-create:last-used"); /** @@ -166,17 +163,8 @@ function useRepositoryHandlers(fs: DialogFormState, repositories: Repository[]) : { repositoryId: undefined, localPath: value, branch: "" }; fs.updateRepository(key, patch); if (isWorkspaceRepo) { - setLocalStorage(STORAGE_KEYS.LAST_REPOSITORY_ID, value); - removeLocalStorage(STORAGE_KEYS.LAST_BRANCH); syncTaskCreateLastUsed({ repository_id: value, branch: null }); - lastUsedDebug(LOCAL_STORAGE_WRITE_EVENT, { - key: "lastRepositoryId", - value, - row_key: key, - }); } else { - removeLocalStorage(STORAGE_KEYS.LAST_REPOSITORY_ID); - removeLocalStorage(STORAGE_KEYS.LAST_BRANCH); syncTaskCreateLastUsed({ repository_id: null, branch: null }); } // Switching the repo invalidates whatever local-status the fresh-branch @@ -189,9 +177,7 @@ function useRepositoryHandlers(fs: DialogFormState, repositories: Repository[]) const handleRowBranchChange = useCallback( (key: string, value: string) => { fs.updateRepository(key, { branch: value }); - setLocalStorage(STORAGE_KEYS.LAST_BRANCH, value); syncTaskCreateLastUsed({ branch: value }); - lastUsedDebug(LOCAL_STORAGE_WRITE_EVENT, { key: "lastBranch", value, row_key: key }); }, [fs], ); @@ -203,18 +189,14 @@ function useProfileAndNameHandlers(fs: DialogFormState) { const handleAgentProfileChange = useCallback( (value: string) => { fs.setAgentProfileId(value); - setLocalStorage(STORAGE_KEYS.LAST_AGENT_PROFILE_ID, value); syncTaskCreateLastUsed({ agent_profile_id: value }); - lastUsedDebug(LOCAL_STORAGE_WRITE_EVENT, { key: "lastAgentProfileId", value }); }, [fs], ); const handleExecutorProfileChange = useCallback( (value: string) => { fs.setExecutorProfileId(value); - setLocalStorage(STORAGE_KEYS.LAST_EXECUTOR_PROFILE_ID, value); syncTaskCreateLastUsed({ executor_profile_id: value }); - lastUsedDebug(LOCAL_STORAGE_WRITE_EVENT, { key: "lastExecutorProfileId", value }); }, [fs], ); @@ -256,8 +238,6 @@ function useGitHubAndFreshBranchHandlers(fs: DialogFormState) { // useRemote when flipping the other way. if (next) { fs.setNoRepository(false); - removeLocalStorage(STORAGE_KEYS.LAST_REPOSITORY_ID); - removeLocalStorage(STORAGE_KEYS.LAST_BRANCH); syncTaskCreateLastUsed({ repository_id: null, branch: null }); } clearFreshBranch(fs); @@ -287,7 +267,6 @@ function useGitHubAndFreshBranchHandlers(fs: DialogFormState) { // non-worktree default (worktree is unworkable in no-repo mode). fs.setExecutorId(""); fs.setExecutorProfileId(""); - removeLocalStorage(STORAGE_KEYS.LAST_EXECUTOR_PROFILE_ID); syncTaskCreateLastUsed({ repository_id: null, branch: null, diff --git a/apps/web/components/task-create-dialog-helpers.test.ts b/apps/web/components/task-create-dialog-helpers.test.ts index 807df8be62..ccb5b7db4f 100644 --- a/apps/web/components/task-create-dialog-helpers.test.ts +++ b/apps/web/components/task-create-dialog-helpers.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it, vi, beforeEach } from "vitest"; import { autoSelectBranch } from "./task-create-dialog-helpers"; -import { STORAGE_KEYS } from "@/lib/settings/constants"; +const STORAGE_KEYS = { LAST_BRANCH: "kandev.dialog.lastBranch" } as const; beforeEach(() => { localStorage.clear(); @@ -21,7 +21,7 @@ describe("autoSelectBranch", () => { expect(setBranch).toHaveBeenCalledWith("main"); }); - it("uses store-backed last-used branch when localStorage is not primed", () => { + it("uses the backend last-used branch before settings finish loading", () => { const setBranch = vi.fn(); autoSelectBranch(branches, setBranch, { @@ -32,7 +32,7 @@ describe("autoSelectBranch", () => { expect(setBranch).toHaveBeenCalledWith("feature"); }); - it("uses store-backed last-used branch when localStorage branch is stale", () => { + it("uses the backend last-used branch when browser storage is stale", () => { const setBranch = vi.fn(); localStorage.setItem(STORAGE_KEYS.LAST_BRANCH, JSON.stringify("deleted")); @@ -52,24 +52,6 @@ describe("autoSelectBranch", () => { expect(setBranch).not.toHaveBeenCalled(); }); - it("defers when localStorage branch is stale and user settings are still loading", () => { - const setBranch = vi.fn(); - localStorage.setItem(STORAGE_KEYS.LAST_BRANCH, JSON.stringify("deleted")); - - autoSelectBranch(branches, setBranch, { userSettingsLoaded: false }); - - expect(setBranch).not.toHaveBeenCalled(); - }); - - it("defers when localStorage branch is valid but user settings are still loading", () => { - const setBranch = vi.fn(); - localStorage.setItem(STORAGE_KEYS.LAST_BRANCH, JSON.stringify("feature")); - - autoSelectBranch(branches, setBranch, { userSettingsLoaded: false }); - - expect(setBranch).not.toHaveBeenCalled(); - }); - it("falls back to preferred branch after user settings have loaded without a valid last-used branch", () => { const setBranch = vi.fn(); @@ -78,6 +60,15 @@ describe("autoSelectBranch", () => { expect(setBranch).toHaveBeenCalledWith("main"); }); + it("ignores a stale localStorage branch after user settings have loaded", () => { + const setBranch = vi.fn(); + localStorage.setItem(STORAGE_KEYS.LAST_BRANCH, JSON.stringify("feature")); + + autoSelectBranch(branches, setBranch, { userSettingsLoaded: true }); + + expect(setBranch).toHaveBeenCalledWith("main"); + }); + it("matches remote branch display names for store-backed last-used branch", () => { const setBranch = vi.fn(); diff --git a/apps/web/components/task-create-dialog-helpers.ts b/apps/web/components/task-create-dialog-helpers.ts index 7f1ead9d88..f8cf68a8c9 100644 --- a/apps/web/components/task-create-dialog-helpers.ts +++ b/apps/web/components/task-create-dialog-helpers.ts @@ -10,8 +10,6 @@ import type { import type { UsePRInfoByURLResult } from "@/hooks/domains/github/use-pr-info-by-url"; import { parseGitHubAnyUrl } from "@/hooks/domains/github/use-pr-info-by-url"; import { selectPreferredBranch } from "@/lib/utils"; -import { getLocalStorage } from "@/lib/local-storage"; -import { STORAGE_KEYS } from "@/lib/settings/constants"; import { createDebugLogger } from "@/lib/debug/log"; import { useContextFilesStore } from "@/lib/state/context-files-store"; import { linkToTask } from "@/lib/links"; @@ -61,16 +59,12 @@ export function autoSelectBranch( setBranch: (value: string) => void, options: { lastUsedBranch?: string | null; userSettingsLoaded?: boolean } = {}, ): void { - const localStorageBranch = getLocalStorage(STORAGE_KEYS.LAST_BRANCH, null); const settingsBranch = options.lastUsedBranch ?? null; - const localStorageValid = isBranchSelectable(branchList, localStorageBranch); const settingsValid = isBranchSelectable(branchList, settingsBranch); if (settingsBranch && settingsValid) { selectionDebug(BRANCH_AUTOPICK_DEBUG, { source: "settings:taskCreateLastUsed", pick: settingsBranch, - local_storage_value: localStorageBranch ?? "-", - local_storage_valid: localStorageValid, branch_count: branchList.length, }); setBranch(settingsBranch); @@ -80,29 +74,14 @@ export function autoSelectBranch( selectionDebug(BRANCH_AUTOPICK_DEBUG, { source: "user-settings-loading", pick: "-", - local_storage_value: localStorageBranch ?? "-", - local_storage_valid: localStorageValid, branch_count: branchList.length, }); return; } - if (localStorageBranch && localStorageValid) { - selectionDebug(BRANCH_AUTOPICK_DEBUG, { - source: "localStorage:lastBranch", - pick: localStorageBranch, - local_storage_value: localStorageBranch, - local_storage_valid: true, - branch_count: branchList.length, - }); - setBranch(localStorageBranch); - return; - } const preferredBranch = selectPreferredBranch(branchList); selectionDebug(BRANCH_AUTOPICK_DEBUG, { source: preferredBranch ? "preferred" : "none", pick: preferredBranch ?? "-", - local_storage_value: localStorageBranch ?? "-", - local_storage_valid: localStorageValid, branch_count: branchList.length, }); if (preferredBranch) setBranch(preferredBranch); diff --git a/apps/web/components/task-create-dialog-multi-repo-guard.ts b/apps/web/components/task-create-dialog-multi-repo-guard.ts index b3a2ac0ed4..775d2a514b 100644 --- a/apps/web/components/task-create-dialog-multi-repo-guard.ts +++ b/apps/web/components/task-create-dialog-multi-repo-guard.ts @@ -1,17 +1,25 @@ "use client"; import { useEffect } from "react"; -import { getLocalStorage } from "@/lib/local-storage"; -import { STORAGE_KEYS } from "@/lib/settings/constants"; import type { Executor } from "@/lib/types/http"; -export function useMultiRepoGuardEffect( - open: boolean, - executorProfileId: string, - setExecutorProfileId: (id: string) => void, - executors: Executor[], - selectedRepoCount: number, -) { +type MultiRepoGuardOptions = { + open: boolean; + executorProfileId: string; + setExecutorProfileId: (id: string) => void; + executors: Executor[]; + selectedRepoCount: number; + lastUsedExecutorProfileId: string | null; +}; + +export function useMultiRepoGuardEffect({ + open, + executorProfileId, + setExecutorProfileId, + executors, + selectedRepoCount, + lastUsedExecutorProfileId, +}: MultiRepoGuardOptions) { useEffect(() => { if (!open || !executorProfileId || executors.length === 0) return; if (selectedRepoCount <= 1) return; @@ -26,8 +34,17 @@ export function useMultiRepoGuardEffect( } if (worktreeProfileIds.length === 0) return; if (profileToType.get(executorProfileId) === "worktree") return; - const lastId = getLocalStorage(STORAGE_KEYS.LAST_EXECUTOR_PROFILE_ID, null); - const pick = lastId && worktreeProfileIds.includes(lastId) ? lastId : worktreeProfileIds[0]; - void Promise.resolve().then(() => setExecutorProfileId(pick)); - }, [open, executorProfileId, executors, selectedRepoCount, setExecutorProfileId]); + const preferredProfileId = + lastUsedExecutorProfileId && worktreeProfileIds.includes(lastUsedExecutorProfileId) + ? lastUsedExecutorProfileId + : worktreeProfileIds[0]; + void Promise.resolve().then(() => setExecutorProfileId(preferredProfileId)); + }, [ + open, + executorProfileId, + executors, + selectedRepoCount, + setExecutorProfileId, + lastUsedExecutorProfileId, + ]); } diff --git a/apps/web/components/task-create-dialog-repository-autopick.test.ts b/apps/web/components/task-create-dialog-repository-autopick.test.ts index 2f81ec2f96..d33d224044 100644 --- a/apps/web/components/task-create-dialog-repository-autopick.test.ts +++ b/apps/web/components/task-create-dialog-repository-autopick.test.ts @@ -3,7 +3,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { useRepositoryAutoSelectEffect } from "./task-create-dialog-repository-autopick"; import type { DialogFormState, TaskRepoRow } from "@/components/task-create-dialog-types"; import type { Repository } from "@/lib/types/http"; -import { STORAGE_KEYS } from "@/lib/settings/constants"; +const STORAGE_KEYS = { LAST_REPOSITORY_ID: "kandev.dialog.lastRepositoryId" } as const; import { readQueuedTaskCreateLastUsedState, resetTaskCreateLastUsedSync, @@ -40,7 +40,6 @@ function makeRepoAutoSelectFs( describe("useRepositoryAutoSelectEffect loading gates", () => { it("waits for store-backed settings before falling back to an empty row", async () => { - window.localStorage.removeItem(STORAGE_KEYS.LAST_REPOSITORY_ID); const setRepositories = vi.fn(); const fs = makeRepoAutoSelectFs([], setRepositories); @@ -69,7 +68,7 @@ describe("useRepositoryAutoSelectEffect loading gates", () => { expect(updater([])).toEqual([{ key: "row-0", repositoryId: "repo-2", branch: "" }]); }); - it("waits for store-backed settings before trusting a valid cached repository id", async () => { + it("ignores a stale cached repository while waiting for backend settings", async () => { window.localStorage.setItem(STORAGE_KEYS.LAST_REPOSITORY_ID, JSON.stringify("repo-1")); const setRepositories = vi.fn(); const fs = makeRepoAutoSelectFs([], setRepositories); @@ -91,10 +90,8 @@ describe("useRepositoryAutoSelectEffect loading gates", () => { expect(setRepositories).not.toHaveBeenCalled(); expect(readQueuedTaskCreateLastUsedState()).toEqual({}); }); -}); -describe("useRepositoryAutoSelectEffect defaults", () => { - it("queues a consumed cached repository id until backend settings catch up", async () => { + it("ignores a stale cached repository after user settings have loaded", async () => { window.localStorage.setItem(STORAGE_KEYS.LAST_REPOSITORY_ID, JSON.stringify("repo-1")); const setRepositories = vi.fn(); const fs = makeRepoAutoSelectFs([], setRepositories); @@ -105,25 +102,19 @@ describe("useRepositoryAutoSelectEffect defaults", () => { true, "ws-1", [makeRepository("repo-1"), makeRepository("repo-2")], - { - lastUsedRepositoryId: null, - userSettingsLoaded: true, - }, + { userSettingsLoaded: true }, ), ); await waitFor(() => expect(setRepositories).toHaveBeenCalled()); const updater = setRepositories.mock.calls[0]![0] as (prev: TaskRepoRow[]) => TaskRepoRow[]; - - expect(updater([])).toEqual([{ key: "row-0", repositoryId: "repo-1", branch: "" }]); - expect(readQueuedTaskCreateLastUsedState()).toMatchObject({ - repositoryId: "repo-1", - branch: null, - }); + expect(updater([])).toEqual([{ key: "row-0", branch: "" }]); + expect(readQueuedTaskCreateLastUsedState()).toEqual({}); }); +}); - it("fills an untouched placeholder row from store-backed settings when localStorage is not primed", async () => { - window.localStorage.removeItem(STORAGE_KEYS.LAST_REPOSITORY_ID); +describe("useRepositoryAutoSelectEffect defaults", () => { + it("fills an untouched placeholder row from backend settings", async () => { const setRepositories = vi.fn(); const fs = makeRepoAutoSelectFs([{ key: "row-0", branch: "" }], setRepositories); @@ -145,8 +136,7 @@ describe("useRepositoryAutoSelectEffect defaults", () => { ]); }); - it("fills an empty repo row list from store-backed settings when localStorage is not primed", async () => { - window.localStorage.removeItem(STORAGE_KEYS.LAST_REPOSITORY_ID); + it("fills an empty repo row list from backend settings", async () => { const setRepositories = vi.fn(); const fs = makeRepoAutoSelectFs([], setRepositories); diff --git a/apps/web/components/task-create-dialog-repository-autopick.ts b/apps/web/components/task-create-dialog-repository-autopick.ts index 36da2d55b6..d544738514 100644 --- a/apps/web/components/task-create-dialog-repository-autopick.ts +++ b/apps/web/components/task-create-dialog-repository-autopick.ts @@ -3,10 +3,7 @@ import { useEffect } from "react"; import type { Repository } from "@/lib/types/http"; import type { DialogFormState, TaskRepoRow } from "@/components/task-create-dialog-types"; -import { getLocalStorage } from "@/lib/local-storage"; -import { STORAGE_KEYS } from "@/lib/settings/constants"; import { createDebugLogger, isDebug } from "@/lib/debug/log"; -import { syncTaskCreateLastUsed } from "@/components/task-create-dialog-handlers"; const selectionDebug = createDebugLogger("task-create:selection"); @@ -14,8 +11,6 @@ type RepositoryAutoPickDecision = { pickId: string | null; source: string; defer: boolean; - localStorageRepoId: string | null; - localStorageValid: boolean; settingsRepoId: string | null; settingsValid: boolean; }; @@ -50,9 +45,6 @@ export function useRepositoryAutoSelectEffect( if (decision.defer) return; const { pickId } = decision; if (rows.length > 0 && !canReplaceEmptyRepositoryPlaceholder(rows, pickId)) return; - if (shouldQueueRepositoryFallback(decision)) { - syncTaskCreateLastUsed({ repository_id: pickId, branch: null }); - } void Promise.resolve().then(() => { setRepositories((prev) => { if (prev.length > 0) return replaceSeededRepositoryRows(prev, pickId); @@ -91,14 +83,10 @@ function decideRepositoryAutoPick( lastUsedRepositoryId?: string | null, userSettingsLoaded = true, ): RepositoryAutoPickDecision { - const localStorageRepoId = getLocalStorage(STORAGE_KEYS.LAST_REPOSITORY_ID, null); const settingsRepoId = lastUsedRepositoryId ?? null; - const localStorageValid = isRepositoryIdValid(localStorageRepoId, repositories); const settingsValid = isRepositoryIdValid(settingsRepoId, repositories); if (settingsRepoId && settingsValid) { return buildRepositoryAutoPickDecision("settings:taskCreateLastUsed", settingsRepoId, { - localStorageRepoId, - localStorageValid, settingsRepoId, settingsValid, }); @@ -106,16 +94,6 @@ function decideRepositoryAutoPick( if (!userSettingsLoaded) { return buildRepositoryAutoPickDecision("user-settings-loading", null, { defer: true, - localStorageRepoId, - localStorageValid, - settingsRepoId, - settingsValid, - }); - } - if (localStorageRepoId && localStorageValid) { - return buildRepositoryAutoPickDecision("localStorage:lastRepositoryId", localStorageRepoId, { - localStorageRepoId, - localStorageValid, settingsRepoId, settingsValid, }); @@ -123,7 +101,7 @@ function decideRepositoryAutoPick( return buildRepositoryAutoPickDecision( repositories.length === 1 ? "single-workspace-repo" : "empty-row", repositories.length === 1 ? repositories[0].id : null, - { localStorageRepoId, localStorageValid, settingsRepoId, settingsValid }, + { settingsRepoId, settingsValid }, ); } @@ -138,8 +116,6 @@ function buildRepositoryAutoPickDecision( pickId, source, defer: fields.defer ?? false, - localStorageRepoId: fields.localStorageRepoId, - localStorageValid: fields.localStorageValid, settingsRepoId: fields.settingsRepoId, settingsValid: fields.settingsValid, }; @@ -149,12 +125,6 @@ function isRepositoryIdValid(repositoryId: string | null, repositories: Reposito return Boolean(repositoryId && repositories.some((r: Repository) => r.id === repositoryId)); } -function shouldQueueRepositoryFallback( - decision: RepositoryAutoPickDecision, -): decision is RepositoryAutoPickDecision & { pickId: string } { - return decision.source === "localStorage:lastRepositoryId" && Boolean(decision.pickId); -} - function logRepositoryAutoPick( workspaceId: string, repoCount: number, @@ -163,8 +133,6 @@ function logRepositoryAutoPick( if (!isDebug()) return; selectionDebug("repository-autopick", { workspace_id: workspaceId, - local_storage_id: decision.localStorageRepoId ?? "-", - local_storage_valid: decision.localStorageValid, settings_id: decision.settingsRepoId ?? "-", settings_valid: decision.settingsValid, repo_count: repoCount, diff --git a/apps/web/components/task-create-dialog-types.ts b/apps/web/components/task-create-dialog-types.ts index c80061c5f0..aacfcfc413 100644 --- a/apps/web/components/task-create-dialog-types.ts +++ b/apps/web/components/task-create-dialog-types.ts @@ -172,15 +172,15 @@ export type TaskCreateEffectsArgs = { workspaceDefaults: Workspace | null | undefined; toast: ReturnType["toast"]; workflows: Array<{ id: string; agent_profile_id?: string }>; - /** Store-backed last-used repository. Used when the localStorage mirror has not been primed yet. */ + /** Backend-owned last-used repository. */ lastUsedRepositoryId?: string | null; /** Whether DB-backed user settings are loaded, or a best-effort fetch has settled. */ userSettingsLoaded?: boolean; - /** Store-backed last-used agent profile. Used when the localStorage mirror has not been primed yet. */ + /** Backend-owned last-used agent profile. */ lastUsedAgentProfileId?: string | null; - /** Store-backed last-used executor profile. Used when the localStorage mirror has not been primed yet. */ + /** Backend-owned last-used executor profile. */ lastUsedExecutorProfileId?: string | null; - /** Store-backed last-used branch. Used when the localStorage mirror has not been primed yet. */ + /** Backend-owned last-used branch. */ lastUsedBranch?: string | null; /** * True when the currently-selected executor is the local-host one (no diff --git a/apps/web/components/task/new-subtask-dialog.tsx b/apps/web/components/task/new-subtask-dialog.tsx index 455b6c2641..7921ea6196 100644 --- a/apps/web/components/task/new-subtask-dialog.tsx +++ b/apps/web/components/task/new-subtask-dialog.tsx @@ -19,8 +19,6 @@ import { useRepositories } from "@/hooks/domains/workspace/use-repositories"; import { useIsUtilityConfigured } from "@/hooks/use-is-utility-configured"; import { useSummarizeSession, type SummarizeSessionResult } from "@/hooks/use-summarize-session"; import { useTaskSessions } from "@/hooks/use-task-sessions"; -import { getLocalStorage } from "@/lib/local-storage"; -import { STORAGE_KEYS } from "@/lib/settings/constants"; import type { ExecutorProfile, ExecutorType, Repository } from "@/lib/types/http"; import type { AgentProfileOption } from "@/lib/state/slices"; import { @@ -113,17 +111,22 @@ function useExecutorProfiles( }, [executors]); } -function useAutoSelectExecutorProfile( +function useExecutorDefault( allProfiles: ExecutorProfile[], executorProfileId: string, - setExecutorProfileId: (v: string) => void, + setExecutorProfileId: (value: string) => void, ) { + const lastUsedExecutorProfileId = useAppStore( + (s) => s.userSettings.taskCreateLastUsed?.executorProfileId ?? null, + ); useEffect(() => { if (executorProfileId || allProfiles.length === 0) return; - const lastId = getLocalStorage(STORAGE_KEYS.LAST_EXECUTOR_PROFILE_ID, null); - const pick = lastId && allProfiles.some((p) => p.id === lastId) ? lastId : allProfiles[0].id; + const pick = + lastUsedExecutorProfileId && allProfiles.some((p) => p.id === lastUsedExecutorProfileId) + ? lastUsedExecutorProfileId + : allProfiles[0].id; setExecutorProfileId(pick); - }, [allProfiles, executorProfileId, setExecutorProfileId]); + }, [allProfiles, executorProfileId, lastUsedExecutorProfileId, setExecutorProfileId]); } /** @@ -253,7 +256,7 @@ function NewSubtaskForm({ const sessionOptions = useSessionOptions(parentTaskId); const allExecutorProfiles = useExecutorProfiles(executors); const executorProfileOptions = useExecutorProfileOptions(allExecutorProfiles); - useAutoSelectExecutorProfile(allExecutorProfiles, fs.executorProfileId, fs.setExecutorProfileId); + useExecutorDefault(allExecutorProfiles, fs.executorProfileId, fs.setExecutorProfileId); const promptZone = useSubtaskPromptZone({ taskTitle: title, inputDisabled: isCreating || isSummarizing, diff --git a/apps/web/e2e/fixtures/docker-test-base.ts b/apps/web/e2e/fixtures/docker-test-base.ts index 0021630b33..a29d274a6a 100644 --- a/apps/web/e2e/fixtures/docker-test-base.ts +++ b/apps/web/e2e/fixtures/docker-test-base.ts @@ -112,28 +112,24 @@ export const dockerTest = backendFixture.extend< testPage: async ({ browser, backend, apiClient, seedData }, use) => { await apiClient.e2eReset(seedData.workspaceId, [seedData.workflowId]); + await apiClient.saveUserSettings({ + workspace_id: seedData.workspaceId, + workflow_filter_id: seedData.workflowId, + task_create_last_used: { + repository_id: seedData.repositoryId, + branch: "main", + agent_profile_id: seedData.agentProfileId, + }, + }); const context = await browser.newContext({ baseURL: backend.frontendUrl }); const page = await context.newPage(); await page.addInitScript( - ({ - backendPort, - repositoryId, - agentProfileId, - }: { - backendPort: string; - repositoryId: string; - agentProfileId: string; - }) => { + ({ backendPort }: { backendPort: string }) => { localStorage.setItem("kandev.onboarding.completed", "true"); - localStorage.setItem("kandev.dialog.lastRepositoryId", JSON.stringify(repositoryId)); - localStorage.setItem("kandev.dialog.lastAgentProfileId", JSON.stringify(agentProfileId)); - localStorage.setItem("kandev.dialog.lastBranch", JSON.stringify("main")); window.__KANDEV_API_PORT = backendPort; }, { backendPort: String((backend as BackendContext).port), - repositoryId: seedData.repositoryId, - agentProfileId: seedData.agentProfileId, }, ); await use(page); diff --git a/apps/web/e2e/fixtures/ssh-test-base.ts b/apps/web/e2e/fixtures/ssh-test-base.ts index aff9a66ea2..4e2fab5864 100644 --- a/apps/web/e2e/fixtures/ssh-test-base.ts +++ b/apps/web/e2e/fixtures/ssh-test-base.ts @@ -98,28 +98,24 @@ export const sshTest = backendFixture.extend< testPage: async ({ browser, backend, apiClient, seedData }, use) => { await apiClient.e2eReset(seedData.workspaceId, [seedData.workflowId]); + await apiClient.saveUserSettings({ + workspace_id: seedData.workspaceId, + workflow_filter_id: seedData.workflowId, + task_create_last_used: { + repository_id: seedData.repositoryId, + branch: "main", + agent_profile_id: seedData.agentProfileId, + }, + }); const context = await browser.newContext({ baseURL: backend.frontendUrl }); const page = await context.newPage(); await page.addInitScript( - ({ - backendPort, - repositoryId, - agentProfileId, - }: { - backendPort: string; - repositoryId: string; - agentProfileId: string; - }) => { + ({ backendPort }: { backendPort: string }) => { localStorage.setItem("kandev.onboarding.completed", "true"); - localStorage.setItem("kandev.dialog.lastRepositoryId", JSON.stringify(repositoryId)); - localStorage.setItem("kandev.dialog.lastAgentProfileId", JSON.stringify(agentProfileId)); - localStorage.setItem("kandev.dialog.lastBranch", JSON.stringify("main")); window.__KANDEV_API_PORT = backendPort; }, { backendPort: String((backend as BackendContext).port), - repositoryId: seedData.repositoryId, - agentProfileId: seedData.agentProfileId, }, ); await use(page); diff --git a/apps/web/e2e/fixtures/test-base.ts b/apps/web/e2e/fixtures/test-base.ts index 8bdbb375eb..db2401ade6 100644 --- a/apps/web/e2e/fixtures/test-base.ts +++ b/apps/web/e2e/fixtures/test-base.ts @@ -169,6 +169,11 @@ export const test = backendFixture.extend< enable_preview_on_click: false, confirm_task_archive: true, sidebar_views: [], + task_create_last_used: { + repository_id: seedData.repositoryId, + branch: "main", + agent_profile_id: seedData.agentProfileId, + }, // Reset to default kanban view. Pipeline-view tests switch this to // "graph2", which persists per-workspace; without this reset the next // test renders cards with data-testid="pipeline-task-" instead of @@ -184,7 +189,7 @@ export const test = backendFixture.extend< console.log(`[browser:${msg.type()}]`, msg.text()); }); } - await setupPage(page, backend, seedData); + await setupPage(page, backend); await use(page); await context.close(); }, @@ -233,28 +238,20 @@ test.beforeEach(async ({ apiClient, seedData }) => { confirm_task_archive: true, sidebar_views: [], kanban_view_mode: "", + task_create_last_used: { + repository_id: seedData.repositoryId, + branch: "main", + agent_profile_id: seedData.agentProfileId, + }, }); }); export { expect } from "@playwright/test"; -async function setupPage(page: Page, backend: BackendContext, seedData: SeedData): Promise { +async function setupPage(page: Page, backend: BackendContext): Promise { await page.addInitScript( - ({ - backendPort, - repositoryId, - agentProfileId, - }: { - backendPort: string; - repositoryId: string; - agentProfileId: string; - }) => { + ({ backendPort }: { backendPort: string }) => { localStorage.setItem("kandev.onboarding.completed", "true"); - // Pre-seed dialog selections so auto-select effects resolve on their - // first render cycle instead of waiting for async API chains. - localStorage.setItem("kandev.dialog.lastRepositoryId", JSON.stringify(repositoryId)); - localStorage.setItem("kandev.dialog.lastAgentProfileId", JSON.stringify(agentProfileId)); - localStorage.setItem("kandev.dialog.lastBranch", JSON.stringify("main")); // Set the window global that getBackendConfig() reads for API/WS connections // (e2e tests run frontend and backend on separate ports, like dev mode) window.__KANDEV_API_PORT = backendPort; @@ -297,8 +294,6 @@ async function setupPage(page: Page, backend: BackendContext, seedData: SeedData }, { backendPort: String(backend.port), - repositoryId: seedData.repositoryId, - agentProfileId: seedData.agentProfileId, }, ); } diff --git a/apps/web/e2e/helpers/api-client.ts b/apps/web/e2e/helpers/api-client.ts index 2017d05e87..9dd36bc3d7 100644 --- a/apps/web/e2e/helpers/api-client.ts +++ b/apps/web/e2e/helpers/api-client.ts @@ -5,6 +5,7 @@ import type { ListWorkflowsResponse, ListWorkflowStepsResponse, TaskSessionState, + TaskCreateLastUsedApi, } from "../../lib/types/http"; import type { Agent, AgentProfile } from "../../lib/types/http-agents"; import type { TaskCIAutomationOptions, TaskCIAutomationPatch } from "../../lib/types/github"; @@ -672,6 +673,7 @@ export class ApiClient { kanban_view_mode?: string; tasks_list_sort?: string; tasks_list_group?: string; + task_create_last_used?: TaskCreateLastUsedApi; voice_mode?: VoiceModeSettings; }): Promise { await this.request("PATCH", "/api/v1/user/settings", settings); diff --git a/apps/web/e2e/tests/task/create-task.spec.ts b/apps/web/e2e/tests/task/create-task.spec.ts index 88c824339f..f0a3132f65 100644 --- a/apps/web/e2e/tests/task/create-task.spec.ts +++ b/apps/web/e2e/tests/task/create-task.spec.ts @@ -44,7 +44,7 @@ test.describe("Task creation", () => { const kanban = new KanbanPage(testPage); await kanban.goto(); - // First: create a task so localStorage selections get persisted + // First: create a task so its selections are persisted to user settings. await kanban.createTaskButton.first().click(); const dialog = testPage.getByTestId("create-task-dialog"); await expect(dialog).toBeVisible(); @@ -149,15 +149,12 @@ test.describe("Task creation", () => { } }); - test("falls back to a valid profile when localStorage holds a stale agent id", async ({ + test("ignores a stale browser profile and selects a valid backend profile", async ({ testPage, }) => { - // Reproduces the "No compatible agent profiles for 'Worktree'" regression - // that fires after a DB reset: localStorage still carries the previous - // run's agent profile UUID, which no longer exists in the fresh DB. - // The dialog must reject the stale id and fall back through the - // workspace default / first-profile chain instead of locking in a - // bogus selection that the compatibility check then rejects. + // A stale value from an older browser build must not participate in + // selection. The dialog uses backend settings and then falls back through + // the workspace default / first-profile chain. await testPage.addInitScript(() => { localStorage.setItem( "kandev.dialog.lastAgentProfileId", diff --git a/apps/web/e2e/tests/task/sidebar-filter.spec.ts b/apps/web/e2e/tests/task/sidebar-filter.spec.ts index 7f5dd66a27..6deef7bfcb 100644 --- a/apps/web/e2e/tests/task/sidebar-filter.spec.ts +++ b/apps/web/e2e/tests/task/sidebar-filter.spec.ts @@ -107,6 +107,14 @@ test.describe("Sidebar filter bar — popover basics", () => { await filters.setClauseTextValue(0, "persist"); await filters.saveAs("Persist View"); await filters.expectActiveViewChip("Persist View"); + await expect + .poll(async () => { + const { settings } = await apiClient.getUserSettings(); + return (settings.sidebar_views as Array<{ name?: string }> | undefined)?.some( + (view) => view.name === "Persist View", + ); + }) + .toBe(true); await testPage.reload(); const session = new SessionPage(testPage); diff --git a/apps/web/hooks/use-sidebar-views-sync.test.ts b/apps/web/hooks/use-sidebar-views-sync.test.ts index 04faa0921a..f9b669c6c6 100644 --- a/apps/web/hooks/use-sidebar-views-sync.test.ts +++ b/apps/web/hooks/use-sidebar-views-sync.test.ts @@ -1,15 +1,12 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { renderHook, waitFor } from "@testing-library/react"; -import { DEFAULT_VIEW_ID } from "@/lib/state/slices/ui/sidebar-view-builtins"; import { useSidebarViewsSync } from "./use-sidebar-views-sync"; const mockToast = vi.fn(); type MockState = { - sidebarViews: { views: Array<{ id: string }>; syncError: string | null }; + sidebarViews: { syncError: string | null }; sidebarTaskPrefs: { syncError?: string | null }; - userSettings: { loaded: boolean }; - migrateLocalViewsToBackend: () => void; clearSidebarSyncError: () => void; clearSidebarTaskPrefsSyncError: () => void; }; @@ -28,10 +25,8 @@ describe("useSidebarViewsSync", () => { beforeEach(() => { mockToast.mockReset(); mockState = { - sidebarViews: { views: [], syncError: null }, + sidebarViews: { syncError: null }, sidebarTaskPrefs: { syncError: null }, - userSettings: { loaded: true }, - migrateLocalViewsToBackend: vi.fn(), clearSidebarSyncError: vi.fn(), clearSidebarTaskPrefsSyncError: vi.fn(), }; @@ -52,21 +47,6 @@ describe("useSidebarViewsSync", () => { }); }); - it("migrates custom local views only once before user settings load", async () => { - mockState.userSettings.loaded = false; - mockState.sidebarViews.views = [{ id: DEFAULT_VIEW_ID }, { id: "custom-view" }]; - - const { rerender } = renderHook(() => useSidebarViewsSync()); - - await waitFor(() => { - expect(mockState.migrateLocalViewsToBackend).toHaveBeenCalledTimes(1); - }); - - rerender(); - - expect(mockState.migrateLocalViewsToBackend).toHaveBeenCalledTimes(1); - }); - it("toasts and clears sidebar view sync errors", async () => { mockState.sidebarViews.syncError = "boom"; diff --git a/apps/web/hooks/use-sidebar-views-sync.ts b/apps/web/hooks/use-sidebar-views-sync.ts index 02cbde6482..3269cc0c9e 100644 --- a/apps/web/hooks/use-sidebar-views-sync.ts +++ b/apps/web/hooks/use-sidebar-views-sync.ts @@ -1,40 +1,16 @@ "use client"; -import { useEffect, useRef } from "react"; +import { useEffect } from "react"; import { useAppStore } from "@/components/state-provider"; import { useToast } from "@/components/toast-provider"; -import { DEFAULT_VIEW_ID } from "@/lib/state/slices/ui/sidebar-view-builtins"; -/** - * Runs one-time migration of locally-stored sidebar views to the backend and - * surfaces sync errors as toasts. Mount once inside the app's ToastProvider. - */ +/** Surfaces sidebar preference sync errors. Mount once inside the app's ToastProvider. */ export function useSidebarViewsSync() { - const views = useAppStore((s) => s.sidebarViews.views); const syncError = useAppStore((s) => s.sidebarViews.syncError); const taskPrefsSyncError = useAppStore((s) => s.sidebarTaskPrefs.syncError); - const userSettingsLoaded = useAppStore((s) => s.userSettings.loaded); - const migrate = useAppStore((s) => s.migrateLocalViewsToBackend); const clearError = useAppStore((s) => s.clearSidebarSyncError); const clearTaskPrefsError = useAppStore((s) => s.clearSidebarTaskPrefsSyncError); const { toast } = useToast(); - const migratedRef = useRef(false); - - useEffect(() => { - if (migratedRef.current) return; - // If user settings hydrated from the server, the views in memory are the - // server's truth (or have already been overlaid by the hydrator). Skip - // migration — the backend already has them, and any future change will - // PATCH through mutateViews. - if (userSettingsLoaded) { - migratedRef.current = true; - return; - } - const hasCustomViews = views.some((v) => v.id !== DEFAULT_VIEW_ID); - if (!hasCustomViews) return; - migratedRef.current = true; - migrate(); - }, [views, userSettingsLoaded, migrate]); useEffect(() => { if (!syncError) return; diff --git a/apps/web/lib/local-storage.ts b/apps/web/lib/local-storage.ts index 018ef5fe57..4759227830 100644 --- a/apps/web/lib/local-storage.ts +++ b/apps/web/lib/local-storage.ts @@ -651,22 +651,6 @@ export function cleanupTaskStorage( setStoredCollapsedSubtaskParents(collapsed.filter((id) => id !== taskId)); } - // Sidebar pin + manual-order arrays. Strip the deleted task so the lists - // don't grow unboundedly across reloads. - const pinned = getStoredPinnedTaskIds(); - if (pinned.includes(taskId)) { - setStoredPinnedTaskIds(pinned.filter((id) => id !== taskId)); - } - const ordered = getStoredOrderedTaskIds(); - if (ordered.includes(taskId)) { - setStoredOrderedTaskIds(ordered.filter((id) => id !== taskId)); - } - - // Per-parent subtask order: drop the deleted task as a parent key, and strip - // it from any other parent's subtask-order list (in case it was a subtask). - const subOrder = getStoredSubtaskOrderByParentId(); - if (pruneSubtaskOrder(subOrder, taskId)) setStoredSubtaskOrderByParentId(subOrder); - // Env-keyed storage — dockview layout + maximize live under task envs. for (const envId of envIds) { removeEnvMaximizeState(envId); @@ -720,112 +704,6 @@ export function removeStoredQuickChatName(sessionId: string): void { setLocalStorage(QUICK_CHAT_NAMES_KEY, all); } -// --- Sidebar filter views (localStorage, global) --- - -const SIDEBAR_VIEWS_KEY = "kandev.sidebar.views"; -const SIDEBAR_ACTIVE_VIEW_KEY = "kandev.sidebar.activeViewId"; -const SIDEBAR_DRAFT_KEY = "kandev.sidebar.draft"; - -// The SidebarView / SidebarViewDraft types aren't structurally assignable to -// JsonValue (the filter clause value is `unknown`), so these wrappers take the -// domain type and do the cast once here — keeps call sites type-safe. -export function getStoredSidebarUserViews(fallback: T): T { - return getLocalStorage(SIDEBAR_VIEWS_KEY, fallback as unknown as JsonValue) as unknown as T; -} - -export function setStoredSidebarUserViews(views: T): void { - setLocalStorage(SIDEBAR_VIEWS_KEY, views as unknown as JsonValue); -} - -export function getStoredSidebarActiveViewId(fallback: string): string { - return getLocalStorage(SIDEBAR_ACTIVE_VIEW_KEY, fallback); -} - -export function setStoredSidebarActiveViewId(id: string): void { - setLocalStorage(SIDEBAR_ACTIVE_VIEW_KEY, id); -} - -export function getStoredSidebarDraft(fallback: T): T { - return getLocalStorage(SIDEBAR_DRAFT_KEY, fallback as unknown as JsonValue) as unknown as T; -} - -export function setStoredSidebarDraft(draft: T): void { - setLocalStorage(SIDEBAR_DRAFT_KEY, draft as unknown as JsonValue); -} - -export function removeStoredSidebarDraft(): void { - removeLocalStorage(SIDEBAR_DRAFT_KEY); -} - -// --- Sidebar task prefs: pin + manual order (localStorage, global) --- - -const SIDEBAR_PINNED_TASKS_KEY = "kandev.sidebar.pinnedTaskIds"; -const SIDEBAR_TASK_ORDER_KEY = "kandev.sidebar.orderedTaskIds"; - -function readStringArray(key: string): string[] { - const raw = getLocalStorage(key, []) as unknown; - if (!Array.isArray(raw)) return []; - return raw.filter((id): id is string => typeof id === "string"); -} - -export function getStoredPinnedTaskIds(): string[] { - return readStringArray(SIDEBAR_PINNED_TASKS_KEY); -} - -export function setStoredPinnedTaskIds(ids: string[]): void { - setLocalStorage(SIDEBAR_PINNED_TASKS_KEY, ids); -} - -export function getStoredOrderedTaskIds(): string[] { - return readStringArray(SIDEBAR_TASK_ORDER_KEY); -} - -export function setStoredOrderedTaskIds(ids: string[]): void { - setLocalStorage(SIDEBAR_TASK_ORDER_KEY, ids); -} - -const SIDEBAR_SUBTASK_ORDER_KEY = "kandev.sidebar.subtaskOrderByParentId"; - -export function getStoredSubtaskOrderByParentId(): Record { - const raw = getLocalStorage>(SIDEBAR_SUBTASK_ORDER_KEY, {}) as unknown; - if (!raw || typeof raw !== "object" || Array.isArray(raw)) return {}; - const out: Record = {}; - for (const [parentId, ids] of Object.entries(raw as Record)) { - if (typeof parentId !== "string" || !Array.isArray(ids)) continue; - const filtered = ids.filter((id): id is string => typeof id === "string"); - if (filtered.length > 0) out[parentId] = filtered; - } - return out; -} - -export function setStoredSubtaskOrderByParentId(map: Record): void { - setLocalStorage(SIDEBAR_SUBTASK_ORDER_KEY, map); -} - -/** - * Strip a task id from a subtask-order map: drop it as a parent key, and - * remove it from every other parent's subtask list (cleaning up the parent - * entry if its list becomes empty). Mutates `map` in place and returns - * `true` if anything changed. Used by both `cleanupTaskStorage` (plain - * object) and `removeTaskFromSidebarPrefs` (Immer draft) to keep the two - * cleanup paths in lockstep. - */ -export function pruneSubtaskOrder(map: Record, taskId: string): boolean { - let changed = false; - if (taskId in map) { - delete map[taskId]; - changed = true; - } - for (const [parentId, ids] of Object.entries(map)) { - if (!ids.includes(taskId)) continue; - const next = ids.filter((id) => id !== taskId); - if (next.length === 0) delete map[parentId]; - else map[parentId] = next; - changed = true; - } - return changed; -} - // AppSidebar collapse/section/width storage helpers live in // `./local-storage-app-sidebar` to keep this module under the line cap. diff --git a/apps/web/lib/settings/constants.ts b/apps/web/lib/settings/constants.ts index 0383ab0afc..d7f631e7e8 100644 --- a/apps/web/lib/settings/constants.ts +++ b/apps/web/lib/settings/constants.ts @@ -1,10 +1,6 @@ export const STORAGE_KEYS = { BACKEND_URL: "kandev.settings.backendUrl", ONBOARDING_COMPLETED: "kandev.onboarding.completed", - LAST_AGENT_PROFILE_ID: "kandev.dialog.lastAgentProfileId", - LAST_REPOSITORY_ID: "kandev.dialog.lastRepositoryId", - LAST_BRANCH: "kandev.dialog.lastBranch", - LAST_EXECUTOR_PROFILE_ID: "kandev.dialog.lastExecutorProfileId", } as const; export const DEFAULT_BACKEND_URL = "http://localhost:38429"; diff --git a/apps/web/lib/state/default-state.ts b/apps/web/lib/state/default-state.ts index fe6b143d90..46e0a6b561 100644 --- a/apps/web/lib/state/default-state.ts +++ b/apps/web/lib/state/default-state.ts @@ -15,6 +15,7 @@ import { defaultSystemState, } from "./slices"; import { getStoredQuickChatNames } from "@/lib/local-storage"; +import { migrateView } from "./slices/ui/ui-slice"; export const defaultState = { kanban: defaultKanbanState.kanban, @@ -147,6 +148,45 @@ function mergeQuickChatState(initialState: Partial): DefaultState[ }; } +function mergeSidebarViewState(initialState: Partial): DefaultState["sidebarViews"] { + const sidebarViews = { ...defaultState.sidebarViews, ...initialState.sidebarViews }; + const userSettings = initialState.userSettings; + const serverViews = userSettings?.sidebarViews?.map(migrateView) ?? []; + if (serverViews.length > 0) sidebarViews.views = serverViews; + + const activeViewId = userSettings?.sidebarActiveViewId; + if (activeViewId && sidebarViews.views.some((view) => view.id === activeViewId)) { + sidebarViews.activeViewId = activeViewId; + } else if (!sidebarViews.views.some((view) => view.id === sidebarViews.activeViewId)) { + sidebarViews.activeViewId = sidebarViews.views[0].id; + } + if (userSettings?.sidebarDraft !== undefined) sidebarViews.draft = userSettings.sidebarDraft; + return sidebarViews; +} + +function mergeSidebarTaskPrefsState( + initialState: Partial, +): DefaultState["sidebarTaskPrefs"] { + const sidebarTaskPrefs = { + ...defaultState.sidebarTaskPrefs, + ...initialState.sidebarTaskPrefs, + }; + const serverPrefs = initialState.userSettings?.sidebarTaskPrefs; + if (!serverPrefs) return sidebarTaskPrefs; + + return { + ...sidebarTaskPrefs, + pinnedTaskIds: [...serverPrefs.pinnedTaskIds], + orderedTaskIds: [...serverPrefs.orderedTaskIds], + subtaskOrderByParentId: Object.fromEntries( + Object.entries(serverPrefs.subtaskOrderByParentId).map(([parentId, taskIds]) => [ + parentId, + [...taskIds], + ]), + ), + }; +} + export function mergeInitialState(initialState?: Partial): DefaultState { if (!initialState) return defaultState; @@ -243,7 +283,8 @@ export function mergeInitialState(initialState?: Partial): Default sessionFailureNotification: initialState.sessionFailureNotification ?? defaultState.sessionFailureNotification, bottomTerminal: { ...defaultState.bottomTerminal, ...initialState.bottomTerminal }, - sidebarViews: { ...defaultState.sidebarViews, ...initialState.sidebarViews }, + sidebarViews: mergeSidebarViewState(initialState), + sidebarTaskPrefs: mergeSidebarTaskPrefsState(initialState), collapsedSubtaskParents: initialState.collapsedSubtaskParents ?? defaultState.collapsedSubtaskParents, }; diff --git a/apps/web/lib/state/hydration/hydrator.test.ts b/apps/web/lib/state/hydration/hydrator.test.ts index 8a563e2e4c..b15400b276 100644 --- a/apps/web/lib/state/hydration/hydrator.test.ts +++ b/apps/web/lib/state/hydration/hydrator.test.ts @@ -127,6 +127,58 @@ describe("mergeInitialState — quick chat name overlay", () => { }); }); +describe("mergeInitialState — sidebar views from boot settings", () => { + it("bridges backend sidebar settings into the UI slice before the store is created", () => { + const result = mergeInitialState({ + userSettings: { + sidebarViews: [ + { + id: "server", + name: "Server", + filters: [], + sort: { key: "state", direction: "asc" }, + group: "none", + collapsedGroups: [], + }, + ], + sidebarActiveViewId: "server", + sidebarDraft: { + baseViewId: "server", + filters: [], + sort: { key: "updatedAt", direction: "desc" }, + group: "workflow", + }, + loaded: true, + }, + } as unknown as Partial); + + expect(result.sidebarViews).toMatchObject({ + views: [{ id: "server", name: "Server" }], + activeViewId: "server", + draft: { baseViewId: "server", group: "workflow" }, + }); + }); + + it("bridges backend task preferences into the UI slice before the store is created", () => { + const result = mergeInitialState({ + userSettings: { + sidebarTaskPrefs: { + pinnedTaskIds: ["task-1"], + orderedTaskIds: ["task-2", "task-1"], + subtaskOrderByParentId: { "task-1": ["subtask-1"] }, + }, + loaded: true, + }, + } as unknown as Partial); + + expect(result.sidebarTaskPrefs).toMatchObject({ + pinnedTaskIds: ["task-1"], + orderedTaskIds: ["task-2", "task-1"], + subtaskOrderByParentId: { "task-1": ["subtask-1"] }, + }); + }); +}); + describe("hydrateState — sidebar views from user settings", () => { it("hydrates active view and draft from backend user settings", () => { const result = produce(makeAppDraft(), (draft: Draft) => { diff --git a/apps/web/lib/state/slices/ui/sidebar-task-prefs-actions.ts b/apps/web/lib/state/slices/ui/sidebar-task-prefs-actions.ts index c64dab50ea..054bba5ef2 100644 --- a/apps/web/lib/state/slices/ui/sidebar-task-prefs-actions.ts +++ b/apps/web/lib/state/slices/ui/sidebar-task-prefs-actions.ts @@ -1,9 +1,3 @@ -import { - pruneSubtaskOrder, - setStoredOrderedTaskIds, - setStoredPinnedTaskIds, - setStoredSubtaskOrderByParentId, -} from "@/lib/local-storage"; import { updateUserSettings } from "@/lib/api/domains/settings-api"; import type { UISlice } from "./types"; @@ -12,6 +6,22 @@ type ImmerSet = (recipe: (draft: UISlice) => void, shouldReplace?: false | undef let sidebarTaskPrefsSync = Promise.resolve(); let sidebarTaskPrefsSyncVersion = 0; +function pruneSubtaskOrder(map: Record, taskId: string): boolean { + let changed = false; + if (taskId in map) { + delete map[taskId]; + changed = true; + } + for (const [parentId, ids] of Object.entries(map)) { + if (!ids.includes(taskId)) continue; + const next = ids.filter((id) => id !== taskId); + if (next.length === 0) delete map[parentId]; + else map[parentId] = next; + changed = true; + } + return changed; +} + function syncSidebarTaskPrefs(prefs: UISlice["sidebarTaskPrefs"], set: ImmerSet) { const syncVersion = ++sidebarTaskPrefsSyncVersion; const payload = { @@ -59,7 +69,6 @@ export function buildSidebarTaskPrefsActions(set: ImmerSet, get: () => UISlice) const idx = list.indexOf(taskId); if (idx === -1) list.push(taskId); else list.splice(idx, 1); - setStoredPinnedTaskIds(list); }); syncSidebarTaskPrefs(get().sidebarTaskPrefs, set); }, @@ -76,7 +85,6 @@ export function buildSidebarTaskPrefsActions(set: ImmerSet, get: () => UISlice) } if (changed) { draft.sidebarTaskPrefs.syncPending = true; - setStoredPinnedTaskIds(list); } }); if (changed) syncSidebarTaskPrefs(get().sidebarTaskPrefs, set); @@ -92,7 +100,6 @@ export function buildSidebarTaskPrefsActions(set: ImmerSet, get: () => UISlice) changed = true; draft.sidebarTaskPrefs.syncPending = true; draft.sidebarTaskPrefs.pinnedTaskIds = next; - setStoredPinnedTaskIds(next); }); if (changed) syncSidebarTaskPrefs(get().sidebarTaskPrefs, set); }, @@ -100,7 +107,6 @@ export function buildSidebarTaskPrefsActions(set: ImmerSet, get: () => UISlice) set((draft) => { draft.sidebarTaskPrefs.syncPending = true; draft.sidebarTaskPrefs.orderedTaskIds = orderedTaskIds; - setStoredOrderedTaskIds(orderedTaskIds); }); syncSidebarTaskPrefs(get().sidebarTaskPrefs, set); }, @@ -110,7 +116,6 @@ export function buildSidebarTaskPrefsActions(set: ImmerSet, get: () => UISlice) const map = draft.sidebarTaskPrefs.subtaskOrderByParentId; if (orderedSubtaskIds.length === 0) delete map[parentTaskId]; else map[parentTaskId] = orderedSubtaskIds; - setStoredSubtaskOrderByParentId(map); }); syncSidebarTaskPrefs(get().sidebarTaskPrefs, set); }, @@ -123,19 +128,16 @@ export function buildSidebarTaskPrefsActions(set: ImmerSet, get: () => UISlice) changed = true; prefs.syncPending = true; prefs.pinnedTaskIds.splice(pinIdx, 1); - setStoredPinnedTaskIds(prefs.pinnedTaskIds); } const orderIdx = prefs.orderedTaskIds.indexOf(taskId); if (orderIdx !== -1) { changed = true; prefs.syncPending = true; prefs.orderedTaskIds.splice(orderIdx, 1); - setStoredOrderedTaskIds(prefs.orderedTaskIds); } if (pruneSubtaskOrder(prefs.subtaskOrderByParentId, taskId)) { changed = true; prefs.syncPending = true; - setStoredSubtaskOrderByParentId(prefs.subtaskOrderByParentId); } }); if (changed) syncSidebarTaskPrefs(get().sidebarTaskPrefs, set); diff --git a/apps/web/lib/state/slices/ui/sidebar-view-actions.ts b/apps/web/lib/state/slices/ui/sidebar-view-actions.ts index 19d2fb1267..465c2d547e 100644 --- a/apps/web/lib/state/slices/ui/sidebar-view-actions.ts +++ b/apps/web/lib/state/slices/ui/sidebar-view-actions.ts @@ -1,10 +1,5 @@ -import { updateUserSettings } from "@/lib/api/domains/settings-api"; -import { - removeStoredSidebarDraft, - setStoredSidebarActiveViewId, - setStoredSidebarDraft, - setStoredSidebarUserViews, -} from "@/lib/local-storage"; +import { updateUserSettingsWithRetry } from "@/lib/user-settings-sync"; +import type { UserSettingsUpdatePayload } from "@/lib/types/http-user-settings"; import type { UISlice, UISliceState } from "./types"; import type { FilterClause, @@ -17,10 +12,6 @@ import { toApiSidebarDraft, toApiSidebarView } from "./sidebar-view-wire"; type ImmerSet = (recipe: (draft: UISlice) => void, shouldReplace?: false | undefined) => void; -function persistUserViews(views: SidebarView[]): void { - setStoredSidebarUserViews(views); -} - function makeId(prefix: string): string { return `${prefix}-${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 8)}`; } @@ -41,6 +32,7 @@ function reorderViewsById( } let viewsSyncRequestId = 0; +const sidebarSettingsQueues = new WeakMap>(); type SidebarSnapshot = { views: SidebarView[]; @@ -56,13 +48,6 @@ function snapshotSidebar(s: UISliceState["sidebarViews"]): SidebarSnapshot { }; } -function writeCacheFromSidebar(s: SidebarSnapshot | UISliceState["sidebarViews"]) { - persistUserViews(s.views); - setStoredSidebarActiveViewId(s.activeViewId); - if (s.draft) setStoredSidebarDraft(s.draft); - else removeStoredSidebarDraft(); -} - function toSidebarSettingsPayload(s: SidebarSnapshot | UISliceState["sidebarViews"]) { return { sidebar_views: s.views.map(toApiSidebarView), @@ -75,6 +60,21 @@ function draftsEqual(a: SidebarViewDraft | null, b: SidebarViewDraft | null): bo return JSON.stringify(a) === JSON.stringify(b); } +function enqueueSidebarSettingsSync( + set: ImmerSet, + payload: UserSettingsUpdatePayload, +): Promise { + const previous = sidebarSettingsQueues.get(set); + const request = previous + ? previous.then(() => updateUserSettingsWithRetry(payload)) + : updateUserSettingsWithRetry(payload); + sidebarSettingsQueues.set( + set, + request.catch(() => undefined), + ); + return request; +} + function syncSidebarViewState( set: ImmerSet, payload: { @@ -82,7 +82,7 @@ function syncSidebarViewState( sidebar_draft: ReturnType | null; }, ) { - updateUserSettings(payload).catch((err) => { + enqueueSidebarSettingsSync(set, payload).catch((err) => { const message = err instanceof Error ? err.message : "Failed to sync sidebar views"; set((draft) => { draft.sidebarViews.syncError = message; @@ -103,9 +103,9 @@ function mutateViews( if (!committed) return; const after = get().sidebarViews; const afterSnapshot = snapshotSidebar(after); - writeCacheFromSidebar(after); const thisRequestId = ++viewsSyncRequestId; - updateUserSettings(toSidebarSettingsPayload(after)).catch((err) => { + const request = enqueueSidebarSettingsSync(set, toSidebarSettingsPayload(after)); + request.catch((err) => { if (thisRequestId !== viewsSyncRequestId) return; const message = err instanceof Error ? err.message : "Failed to sync sidebar views"; set((draft) => { @@ -118,7 +118,6 @@ function mutateViews( } draft.sidebarViews.syncError = message; }); - writeCacheFromSidebar(get().sidebarViews); }); } @@ -131,8 +130,6 @@ function buildSidebarLocalActions(set: ImmerSet, get: () => UISlice) { committed = true; draft.sidebarViews.activeViewId = viewId; draft.sidebarViews.draft = null; - setStoredSidebarActiveViewId(viewId); - removeStoredSidebarDraft(); }); if (!committed) return; syncSidebarViewState(set, { sidebar_active_view_id: viewId, sidebar_draft: null }); @@ -160,7 +157,6 @@ function buildSidebarLocalActions(set: ImmerSet, get: () => UISlice) { group: patch.group ?? current.group, }; draft.sidebarViews.draft = next; - setStoredSidebarDraft(next); }); if (!committed) return; const { activeViewId, draft } = get().sidebarViews; @@ -172,7 +168,6 @@ function buildSidebarLocalActions(set: ImmerSet, get: () => UISlice) { discardSidebarDraft: () => { set((draft) => { draft.sidebarViews.draft = null; - removeStoredSidebarDraft(); }); syncSidebarViewState(set, { sidebar_active_view_id: get().sidebarViews.activeViewId, @@ -183,27 +178,6 @@ function buildSidebarLocalActions(set: ImmerSet, get: () => UISlice) { set((draft) => { draft.sidebarViews.syncError = null; }), - toggleSidebarGroupCollapsed: (viewId: string, groupKey: string) => { - set((draft) => { - const view = draft.sidebarViews.views.find((v) => v.id === viewId); - if (!view) return; - const idx = view.collapsedGroups.indexOf(groupKey); - if (idx === -1) view.collapsedGroups.push(groupKey); - else view.collapsedGroups.splice(idx, 1); - }); - persistUserViews(get().sidebarViews.views); - }, - migrateLocalViewsToBackend: () => { - const sidebar = get().sidebarViews; - const thisRequestId = ++viewsSyncRequestId; - updateUserSettings(toSidebarSettingsPayload(sidebar)).catch((err) => { - if (thisRequestId !== viewsSyncRequestId) return; - const message = err instanceof Error ? err.message : "Failed to sync sidebar views"; - set((draft) => { - draft.sidebarViews.syncError = message; - }); - }); - }, }; } @@ -211,6 +185,14 @@ function buildSidebarBackendActions(set: ImmerSet, get: () => UISlice) { const mv = (mutate: (s: UISliceState["sidebarViews"]) => boolean | void) => mutateViews(set, get, mutate); return { + toggleSidebarGroupCollapsed: (viewId: string, groupKey: string) => + mv((s) => { + const view = s.views.find((v) => v.id === viewId); + if (!view) return false; + const idx = view.collapsedGroups.indexOf(groupKey); + if (idx === -1) view.collapsedGroups.push(groupKey); + else view.collapsedGroups.splice(idx, 1); + }), saveSidebarDraftAs: (name: string) => mv((s) => { if (!s.draft) return false; diff --git a/apps/web/lib/state/slices/ui/types.ts b/apps/web/lib/state/slices/ui/types.ts index c5b0d056d3..1868a0699c 100644 --- a/apps/web/lib/state/slices/ui/types.ts +++ b/apps/web/lib/state/slices/ui/types.ts @@ -231,7 +231,6 @@ export type UISliceActions = { toggleSubtaskCollapsed: (parentTaskId: string) => void; clearSidebarSyncError: () => void; clearSidebarTaskPrefsSyncError: () => void; - migrateLocalViewsToBackend: () => void; setKanbanPreviewedTaskId: (taskId: string | null) => void; togglePinnedTask: (taskId: string) => void; /** Pin every id that isn't already pinned (bulk "Pin" for multi-select). */ diff --git a/apps/web/lib/state/slices/ui/ui-slice.test.ts b/apps/web/lib/state/slices/ui/ui-slice.test.ts index 8772971da2..0a46c6c95c 100644 --- a/apps/web/lib/state/slices/ui/ui-slice.test.ts +++ b/apps/web/lib/state/slices/ui/ui-slice.test.ts @@ -3,9 +3,9 @@ import { create, type StoreApi, type UseBoundStore } from "zustand"; import { waitFor } from "@testing-library/react"; import { immer } from "zustand/middleware/immer"; import { updateUserSettings } from "@/lib/api/domains/settings-api"; -import { createUISlice } from "./ui-slice"; +import { createUISlice, migrateView } from "./ui-slice"; import { APP_SIDEBAR_EXPANDED_WIDTH } from "@/components/app-sidebar/app-sidebar-constants"; -import type { SidebarViewDraft } from "./sidebar-view-types"; +import type { SidebarView, SidebarViewDraft } from "./sidebar-view-types"; import type { UISlice } from "./types"; import { getStoredAcknowledgedAgentErrors, @@ -28,14 +28,12 @@ type UIStore = UseBoundStore>; const KEY = "kandev.sidebar.collapsedSubtasks"; const TASK_A = "task-a"; const TASK_B = "task-b"; -const SIDEBAR_VIEWS_KEY = "kandev.sidebar.views"; -const SIDEBAR_ACTIVE_VIEW_KEY = "kandev.sidebar.activeViewId"; -const SIDEBAR_DRAFT_KEY = "kandev.sidebar.draft"; const BACKEND_DOWN = "backend down"; +const RENAME_FAILED = "rename failed"; const PINNED_KEY = "kandev.sidebar.pinnedTaskIds"; const ORDER_KEY = "kandev.sidebar.orderedTaskIds"; -function makeSidebarView(id: string, name: string) { +function makeSidebarView(id: string, name: string): SidebarView { return { id, name, @@ -46,6 +44,18 @@ function makeSidebarView(id: string, name: string) { }; } +describe("migrateView", () => { + it("retains all supported boolean filter dimensions", () => { + const view = makeSidebarView("view-a", "View A"); + view.filters = [ + { id: "has-pr", dimension: "hasPR", op: "is", value: true }, + { id: "issue-watch", dimension: "isIssueWatch", op: "is", value: true }, + ]; + + expect(migrateView(view).filters).toEqual(view.filters); + }); +}); + describe("toggleSubtaskCollapsed", () => { beforeEach(() => { window.sessionStorage.clear(); @@ -94,26 +104,24 @@ describe("sidebar task prefs (pin + manual order)", () => { } as Awaited>); }); - it("hydrates pinned + ordered from localStorage", () => { + it("ignores stale pinned + ordered values in localStorage", () => { window.localStorage.setItem(PINNED_KEY, JSON.stringify(["t1"])); window.localStorage.setItem(ORDER_KEY, JSON.stringify(["t2", "t1"])); const store = makeStore(); - expect(store.getState().sidebarTaskPrefs.pinnedTaskIds).toEqual(["t1"]); - expect(store.getState().sidebarTaskPrefs.orderedTaskIds).toEqual(["t2", "t1"]); + expect(store.getState().sidebarTaskPrefs.pinnedTaskIds).toEqual([]); + expect(store.getState().sidebarTaskPrefs.orderedTaskIds).toEqual([]); }); - it("togglePinnedTask adds, removes, and persists", () => { + it("togglePinnedTask adds and removes tasks", () => { const store = makeStore(); store.getState().togglePinnedTask("t1"); expect(store.getState().sidebarTaskPrefs.pinnedTaskIds).toEqual(["t1"]); - expect(JSON.parse(window.localStorage.getItem(PINNED_KEY) ?? "null")).toEqual(["t1"]); store.getState().togglePinnedTask("t2"); expect(store.getState().sidebarTaskPrefs.pinnedTaskIds).toEqual(["t1", "t2"]); store.getState().togglePinnedTask("t1"); expect(store.getState().sidebarTaskPrefs.pinnedTaskIds).toEqual(["t2"]); - expect(JSON.parse(window.localStorage.getItem(PINNED_KEY) ?? "null")).toEqual(["t2"]); }); it("pinTasks pins all given ids without unpinning existing ones", () => { @@ -123,11 +131,6 @@ describe("sidebar task prefs (pin + manual order)", () => { store.getState().pinTasks(["t1", "t2", "t3"]); // t1 already pinned stays pinned; t2/t3 added. expect(store.getState().sidebarTaskPrefs.pinnedTaskIds).toEqual(["t1", "t2", "t3"]); - expect(JSON.parse(window.localStorage.getItem(PINNED_KEY) ?? "null")).toEqual([ - "t1", - "t2", - "t3", - ]); }); it("unpinTasks removes all given ids and leaves other pinned tasks alone", () => { @@ -137,21 +140,18 @@ describe("sidebar task prefs (pin + manual order)", () => { store.getState().unpinTasks(["t1", "t3"]); expect(store.getState().sidebarTaskPrefs.pinnedTaskIds).toEqual(["t2"]); - expect(JSON.parse(window.localStorage.getItem(PINNED_KEY) ?? "null")).toEqual(["t2"]); }); - it("setSidebarTaskOrder replaces and persists", () => { + it("setSidebarTaskOrder replaces the order", () => { const store = makeStore(); store.getState().setSidebarTaskOrder(["a", "b", "c"]); expect(store.getState().sidebarTaskPrefs.orderedTaskIds).toEqual(["a", "b", "c"]); - expect(JSON.parse(window.localStorage.getItem(ORDER_KEY) ?? "null")).toEqual(["a", "b", "c"]); store.getState().setSidebarTaskOrder(["c", "a"]); expect(store.getState().sidebarTaskPrefs.orderedTaskIds).toEqual(["c", "a"]); - expect(JSON.parse(window.localStorage.getItem(ORDER_KEY) ?? "null")).toEqual(["c", "a"]); }); - it("removeTaskFromSidebarPrefs strips the id from both arrays and persists", () => { + it("removeTaskFromSidebarPrefs strips the id from both arrays", () => { const store = makeStore(); store.getState().togglePinnedTask("t1"); store.getState().togglePinnedTask("t2"); @@ -161,22 +161,17 @@ describe("sidebar task prefs (pin + manual order)", () => { expect(store.getState().sidebarTaskPrefs.pinnedTaskIds).toEqual(["t2"]); expect(store.getState().sidebarTaskPrefs.orderedTaskIds).toEqual(["t2", "t3"]); - expect(JSON.parse(window.localStorage.getItem(PINNED_KEY) ?? "null")).toEqual(["t2"]); - expect(JSON.parse(window.localStorage.getItem(ORDER_KEY) ?? "null")).toEqual(["t2", "t3"]); // Subsequent togglePinnedTask must NOT bring "t1" back from a stale draft. store.getState().togglePinnedTask("t3"); expect(store.getState().sidebarTaskPrefs.pinnedTaskIds).toEqual(["t2", "t3"]); - expect(JSON.parse(window.localStorage.getItem(PINNED_KEY) ?? "null")).toEqual(["t2", "t3"]); }); it("removeTaskFromSidebarPrefs is a no-op for unknown ids", () => { const store = makeStore(); store.getState().togglePinnedTask("t1"); - const before = window.localStorage.getItem(PINNED_KEY); store.getState().removeTaskFromSidebarPrefs("ghost"); expect(store.getState().sidebarTaskPrefs.pinnedTaskIds).toEqual(["t1"]); - expect(window.localStorage.getItem(PINNED_KEY)).toBe(before); }); }); @@ -249,7 +244,9 @@ describe("sidebar view sync rollback", () => { const store = makeStore(); seedViews(store); vi.mocked(updateUserSettings) - .mockRejectedValueOnce(new Error("rename failed")) + .mockRejectedValueOnce(new Error(RENAME_FAILED)) + .mockRejectedValueOnce(new Error(RENAME_FAILED)) + .mockRejectedValueOnce(new Error(RENAME_FAILED)) .mockResolvedValueOnce({ settings: {}, } as Awaited>); @@ -258,7 +255,7 @@ describe("sidebar view sync rollback", () => { store.getState().setSidebarActiveView("view-b"); await waitFor(() => { - expect(store.getState().sidebarViews.syncError).toBe("rename failed"); + expect(store.getState().sidebarViews.syncError).toBe(RENAME_FAILED); }); expect(store.getState().sidebarViews.activeViewId).toBe("view-b"); @@ -275,23 +272,18 @@ describe("setSubtaskOrder", () => { window.localStorage.clear(); }); - it("hydrates subtaskOrderByParentId from localStorage", () => { + it("ignores stale subtask order in localStorage", () => { window.localStorage.setItem(SUB_KEY, JSON.stringify({ [PARENT_A]: ["c1", "c2"] })); const store = makeStore(); - expect(store.getState().sidebarTaskPrefs.subtaskOrderByParentId).toEqual({ - [PARENT_A]: ["c1", "c2"], - }); + expect(store.getState().sidebarTaskPrefs.subtaskOrderByParentId).toEqual({}); }); - it("setSubtaskOrder writes per-parent order and persists", () => { + it("setSubtaskOrder writes per-parent order", () => { const store = makeStore(); store.getState().setSubtaskOrder(PARENT_A, ["c2", "c1"]); expect(store.getState().sidebarTaskPrefs.subtaskOrderByParentId).toEqual({ [PARENT_A]: ["c2", "c1"], }); - expect(JSON.parse(window.localStorage.getItem(SUB_KEY) ?? "null")).toEqual({ - [PARENT_A]: ["c2", "c1"], - }); store.getState().setSubtaskOrder(PARENT_B, ["d1"]); expect(store.getState().sidebarTaskPrefs.subtaskOrderByParentId).toEqual({ @@ -305,7 +297,6 @@ describe("setSubtaskOrder", () => { store.getState().setSubtaskOrder(PARENT_A, ["c1"]); store.getState().setSubtaskOrder(PARENT_A, []); expect(store.getState().sidebarTaskPrefs.subtaskOrderByParentId).toEqual({}); - expect(JSON.parse(window.localStorage.getItem(SUB_KEY) ?? "null")).toEqual({}); }); it("removeTaskFromSidebarPrefs drops the task as a parent key and from sibling lists", () => { @@ -332,7 +323,6 @@ describe("setSubtaskOrder", () => { store.getState().setSubtaskOrder(PARENT_A, ["c1"]); store.getState().removeTaskFromSidebarPrefs("c1"); expect(store.getState().sidebarTaskPrefs.subtaskOrderByParentId).toEqual({}); - expect(JSON.parse(window.localStorage.getItem(SUB_KEY) ?? "null")).toEqual({}); }); }); @@ -515,18 +505,6 @@ describe("reorderSidebarViews", () => { store.getState().reorderSidebarViews("two", "one"); expect(store.getState().sidebarViews.views.map((v) => v.id)).toEqual(["all", "two", "one"]); - expect(JSON.parse(window.localStorage.getItem(SIDEBAR_VIEWS_KEY) ?? "[]")).toEqual( - expect.arrayContaining([ - expect.objectContaining({ id: "all" }), - expect.objectContaining({ id: "two" }), - expect.objectContaining({ id: "one" }), - ]), - ); - expect( - JSON.parse(window.localStorage.getItem(SIDEBAR_VIEWS_KEY) ?? "[]").map( - (v: { id: string }) => v.id, - ), - ).toEqual(["all", "two", "one"]); expect(updateUserSettings).toHaveBeenCalledWith({ sidebar_views: [ expect.objectContaining({ id: "all" }), @@ -614,8 +592,6 @@ describe("sidebar view backend state", () => { store.getState().setSidebarActiveView("mine"); expect(store.getState().sidebarViews.activeViewId).toBe("mine"); - expect(window.localStorage.getItem(SIDEBAR_ACTIVE_VIEW_KEY)).toBe(JSON.stringify("mine")); - expect(window.localStorage.getItem(SIDEBAR_DRAFT_KEY)).toBeNull(); expect(updateUserSettings).toHaveBeenCalledWith({ sidebar_active_view_id: "mine", sidebar_draft: null, diff --git a/apps/web/lib/state/slices/ui/ui-slice.ts b/apps/web/lib/state/slices/ui/ui-slice.ts index 5287c76551..d06da0cc6d 100644 --- a/apps/web/lib/state/slices/ui/ui-slice.ts +++ b/apps/web/lib/state/slices/ui/ui-slice.ts @@ -1,16 +1,9 @@ import type { StateCreator } from "zustand"; import { getStoredCollapsedSubtaskParents, - getStoredOrderedTaskIds, - getStoredPinnedTaskIds, - getStoredSidebarActiveViewId, - getStoredSidebarDraft, - getStoredSidebarUserViews, - getStoredSubtaskOrderByParentId, setLocalStorage, setStoredCollapsedSubtaskParents, setStoredQuickChatName, - setStoredSidebarUserViews, } from "@/lib/local-storage"; import { buildDismissedAgentErrors } from "./dismissed-agent-errors-actions"; import { @@ -21,21 +14,13 @@ import { import { APP_SIDEBAR_EXPANDED_WIDTH } from "@/components/app-sidebar/app-sidebar-constants"; import { buildSidebarTaskPrefsActions } from "./sidebar-task-prefs-actions"; import { buildSidebarViewActions } from "./sidebar-view-actions"; -import { DEFAULT_ACTIVE_VIEW_ID, DEFAULT_VIEW } from "./sidebar-view-builtins"; -import type { SidebarView, SidebarViewDraft, SortSpec } from "./sidebar-view-types"; +import { DEFAULT_VIEW } from "./sidebar-view-builtins"; +import type { SidebarView, SortSpec } from "./sidebar-view-types"; import type { SystemHealthResponse } from "@/lib/types/health"; import type { ActiveDocument, UISlice, UISliceState } from "./types"; -function loadSidebarState(): UISliceState["sidebarViews"] { - let views = getStoredSidebarUserViews([]).map(migrateView); - if (views.length === 0) { - views = [DEFAULT_VIEW]; - } - setStoredSidebarUserViews(views); - const storedActive = getStoredSidebarActiveViewId(DEFAULT_ACTIVE_VIEW_ID); - const activeViewId = views.some((v) => v.id === storedActive) ? storedActive : views[0].id; - const draft = getStoredSidebarDraft(null); - return { views, activeViewId, draft, syncError: null }; +function createDefaultSidebarState(): UISliceState["sidebarViews"] { + return { views: [DEFAULT_VIEW], activeViewId: DEFAULT_VIEW.id, draft: null, syncError: null }; } export const KNOWN_DIMENSIONS = new Set([ @@ -46,7 +31,9 @@ export const KNOWN_DIMENSIONS = new Set([ "executorType", "repository", "hasDiff", + "hasPR", "isPRReview", + "isIssueWatch", "titleMatch", ]); @@ -93,7 +80,7 @@ export const defaultUIState: UISliceState = { sessionFailureNotification: null, taskDeletedNotification: null, bottomTerminal: { isOpen: false, pendingCommand: null }, - sidebarViews: loadSidebarState(), + sidebarViews: createDefaultSidebarState(), collapsedSubtaskParents: [], kanbanPreviewedTaskId: null, sidebarTaskPrefs: { pinnedTaskIds: [], orderedTaskIds: [], subtaskOrderByParentId: {} }, @@ -379,11 +366,7 @@ export const createUISlice: StateCreator { + it("retains sidebar boot settings after slice initialization", () => { + const store = createAppStore({ + userSettings: { + sidebarViews: [ + { + id: "server", + name: "Server", + filters: [], + sort: { key: "state", direction: "asc" }, + group: "none", + collapsedGroups: [], + }, + ], + sidebarActiveViewId: "server", + sidebarTaskPrefs: { + pinnedTaskIds: ["task-1"], + orderedTaskIds: ["task-1"], + subtaskOrderByParentId: {}, + }, + loaded: true, + }, + } as unknown as Partial); + + expect(store.getState().sidebarViews.activeViewId).toBe("server"); + expect(store.getState().sidebarTaskPrefs.pinnedTaskIds).toEqual(["task-1"]); + }); +}); diff --git a/apps/web/lib/state/store.ts b/apps/web/lib/state/store.ts index f54018132e..d883cc5ba4 100644 --- a/apps/web/lib/state/store.ts +++ b/apps/web/lib/state/store.ts @@ -530,7 +530,6 @@ export type AppState = { toggleSubtaskCollapsed: UIA["toggleSubtaskCollapsed"]; clearSidebarSyncError: UIA["clearSidebarSyncError"]; clearSidebarTaskPrefsSyncError: UIA["clearSidebarTaskPrefsSyncError"]; - migrateLocalViewsToBackend: UIA["migrateLocalViewsToBackend"]; setKanbanPreviewedTaskId: UIA["setKanbanPreviewedTaskId"]; togglePinnedTask: UIA["togglePinnedTask"]; pinTasks: UIA["pinTasks"]; diff --git a/apps/web/lib/user-settings-sync-failure.ts b/apps/web/lib/user-settings-sync-failure.ts deleted file mode 100644 index 669639bf27..0000000000 --- a/apps/web/lib/user-settings-sync-failure.ts +++ /dev/null @@ -1,18 +0,0 @@ -export function hasUserSettingsSyncFailure(storageKey: string): boolean { - if (typeof window === "undefined") return false; - try { - return window.localStorage.getItem(storageKey) === "1"; - } catch { - return false; - } -} - -export function setUserSettingsSyncFailure(storageKey: string, failed: boolean): void { - if (typeof window === "undefined") return; - try { - if (failed) window.localStorage.setItem(storageKey, "1"); - else window.localStorage.removeItem(storageKey); - } catch { - /* ignore storage failures */ - } -} diff --git a/apps/web/lib/user-settings-sync.test.ts b/apps/web/lib/user-settings-sync.test.ts index 8bbe3f3bcb..7c9ec4ebbe 100644 --- a/apps/web/lib/user-settings-sync.test.ts +++ b/apps/web/lib/user-settings-sync.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { waitFor } from "@testing-library/react"; +import { ApiError } from "@/lib/api/client"; import { updateUserSettings } from "@/lib/api/domains/settings-api"; import { createQueuedUserSettingsSync } from "./user-settings-sync"; @@ -26,7 +27,7 @@ describe("createQueuedUserSettingsSync", () => { vi.mocked(updateUserSettings) .mockReturnValueOnce(first.promise) .mockResolvedValueOnce({ settings: {} } as Awaited>); - const sync = createQueuedUserSettingsSync("sync-key", (value) => ({ + const sync = createQueuedUserSettingsSync((value) => ({ preferred_shell: value, })); @@ -45,4 +46,34 @@ describe("createQueuedUserSettingsSync", () => { expect(updateUserSettings).toHaveBeenLastCalledWith({ preferred_shell: "zsh" }); }); }); + + it("retries a transient settings write before processing the next payload", async () => { + vi.mocked(updateUserSettings) + .mockRejectedValueOnce(new Error("temporary failure")) + .mockResolvedValueOnce({ settings: {} } as Awaited>) + .mockResolvedValueOnce({ settings: {} } as Awaited>); + const sync = createQueuedUserSettingsSync((value) => ({ + preferred_shell: value, + })); + + void sync("bash"); + void sync("zsh"); + + await waitFor(() => { + expect(updateUserSettings).toHaveBeenNthCalledWith(1, { preferred_shell: "bash" }); + expect(updateUserSettings).toHaveBeenNthCalledWith(2, { preferred_shell: "bash" }); + expect(updateUserSettings).toHaveBeenNthCalledWith(3, { preferred_shell: "zsh" }); + }); + }); + + it("does not retry an API error", async () => { + vi.mocked(updateUserSettings).mockRejectedValueOnce(new ApiError("invalid settings", 400, {})); + const sync = createQueuedUserSettingsSync((value) => ({ + preferred_shell: value, + })); + + await sync("bash"); + + expect(updateUserSettings).toHaveBeenCalledTimes(1); + }); }); diff --git a/apps/web/lib/user-settings-sync.ts b/apps/web/lib/user-settings-sync.ts index 3ca1b57cd8..7630e28bac 100644 --- a/apps/web/lib/user-settings-sync.ts +++ b/apps/web/lib/user-settings-sync.ts @@ -1,9 +1,32 @@ import { updateUserSettings } from "@/lib/api/domains/settings-api"; -import { setUserSettingsSyncFailure } from "@/lib/user-settings-sync-failure"; +import { ApiError } from "@/lib/api/client"; import type { UserSettingsUpdatePayload } from "@/lib/types/http-user-settings"; +const MAX_SYNC_ATTEMPTS = 3; +const BASE_SYNC_RETRY_DELAY_MS = 100; + +export async function updateUserSettingsWithRetry( + payload: UserSettingsUpdatePayload, +): Promise { + let lastError: unknown; + for (let attempt = 0; attempt < MAX_SYNC_ATTEMPTS; attempt += 1) { + try { + await updateUserSettings(payload); + return; + } catch (error) { + if (error instanceof ApiError) throw error; + lastError = error; + if (attempt < MAX_SYNC_ATTEMPTS - 1) { + await new Promise((resolve) => + setTimeout(resolve, BASE_SYNC_RETRY_DELAY_MS * 2 ** attempt), + ); + } + } + } + throw lastError; +} + export function createQueuedUserSettingsSync( - syncFailedKey: string, buildPayload: (value: T) => UserSettingsUpdatePayload, ): (value: T) => Promise { let queue = Promise.resolve(); @@ -11,15 +34,8 @@ export function createQueuedUserSettingsSync( const payload = buildPayload(value); queue = queue .catch(() => undefined) - .then(() => - updateUserSettings(payload) - .then(() => { - setUserSettingsSyncFailure(syncFailedKey, false); - }) - .catch(() => { - setUserSettingsSyncFailure(syncFailedKey, true); - }), - ); + .then(() => updateUserSettingsWithRetry(payload)) + .catch(() => undefined); return queue; }; } diff --git a/apps/web/lib/ws/handlers/tasks.ts b/apps/web/lib/ws/handlers/tasks.ts index be674cf5b5..6237b8f7ce 100644 --- a/apps/web/lib/ws/handlers/tasks.ts +++ b/apps/web/lib/ws/handlers/tasks.ts @@ -405,9 +405,7 @@ export function registerTasksHandlers(store: StoreApi): WsHandlers { ), ); cleanupTaskStorage(deletedId, sessionIds, envIds); - // Keep the in-memory sidebar pin/order arrays in sync — without this, - // a later togglePinnedTask / setSidebarTaskOrder would persist the - // stale state (still containing the deleted ID) back to localStorage. + // Remove the deleted task before the next sidebar preference PATCH. currentState.removeTaskFromSidebarPrefs(deletedId); for (const sid of sessionIds) { useContextFilesStore.getState().clearSession(sid); diff --git a/docs/decisions/0028-task-create-last-used-source-of-truth.md b/docs/decisions/0028-task-create-last-used-source-of-truth.md index fb4e8334ec..fe7546c20e 100644 --- a/docs/decisions/0028-task-create-last-used-source-of-truth.md +++ b/docs/decisions/0028-task-create-last-used-source-of-truth.md @@ -1,6 +1,6 @@ # 0028: Backend-Owned Task-Create Last-Used Preferences -**Status:** accepted +**Status:** accepted; browser-cache portion superseded by 0041 **Date:** 2026-06-29 **Area:** backend, frontend @@ -10,7 +10,7 @@ The task-create dialog restores the user's last repository, branch, agent profil ## Decision -Backend user settings are the durable source of truth for `task_create_last_used`. Successful task creation records the final values used by the backend. Browser localStorage remains a cache and pending-write bridge, but dialog auto-selection must wait for backend settings to load before trusting cached values. +Backend user settings are the durable source of truth for `task_create_last_used`. Successful task creation records the final values used by the backend. The original browser-cache compatibility path was removed by ADR 0041 after the migration window. Dialog auto-selection now uses backend settings only; an in-memory queued overlay may bridge an in-flight task-create response without becoming a second durable store. Task-create preference writes use targeted JSON updates under `users.settings.task_create_last_used`, and broad user-settings writes preserve the current task-create preference instead of rewriting it from a stale settings blob. diff --git a/docs/decisions/0030-workspace-scoped-integration-settings.md b/docs/decisions/0030-workspace-scoped-integration-settings.md index 27b7e591ca..0baa2af216 100644 --- a/docs/decisions/0030-workspace-scoped-integration-settings.md +++ b/docs/decisions/0030-workspace-scoped-integration-settings.md @@ -63,9 +63,8 @@ task-creating pollers. current behavior until a workspace changes its GitHub scope. - Workspace-scoped GitHub searches have cache keys that include the workspace scope so scoped and unscoped result pages do not share cached data. -- Default query presets migrate opportunistically from the older global - browser/user-settings path into the first workspace where the user opens - GitHub settings or the `/github` page. +- The temporary opportunistic browser/user-settings migration for default + query presets was removed by ADR 0041 after the migration window. - Task creation repo/branch pickers remain governed by workspace repositories; GitHub repository scope only affects GitHub integration surfaces. - Config/status APIs must either require `workspace_id` or derive it from the diff --git a/docs/decisions/0041-backend-owned-portable-user-settings.md b/docs/decisions/0041-backend-owned-portable-user-settings.md new file mode 100644 index 0000000000..8299a8d9b7 --- /dev/null +++ b/docs/decisions/0041-backend-owned-portable-user-settings.md @@ -0,0 +1,44 @@ +# 0041: Backend-Owned Portable User Settings + +**Status:** accepted +**Date:** 2026-07-15 +**Area:** frontend, backend + +## Context + +Portable user preferences were migrated from browser storage to backend user +settings with temporary local fallbacks, dual writes, and retry markers. Those +paths remained long enough for active local installations to run a migration +release, but keeping them allowed stale browser values to compete with the +backend and complicated hydration. + +## Decision + +Backend user and workspace settings are the only durable source of truth for +portable preferences. The frontend may update in-memory state optimistically, +but it does not hydrate or persist task filters, sidebar views and ordering, +integration presets, or task-create last-used choices through localStorage or +sessionStorage. + +Browser storage remains appropriate for explicitly device-local UI state and +transient drafts. User-settings PATCH semantics remain unchanged: omitted +fields are unchanged, explicit `null` clears nullable fields where supported, +and an empty array is an explicit empty value. + +## Consequences + +- Stale browser values cannot override backend settings during hydration. +- Failed optimistic writes are not reconstructed from browser retry markers; + the backend value wins on the next load. +- Cross-device behavior is consistent because portable settings have one + durable owner. +- Device-local layout, collapse, pane-size, and transient draft storage remain + unaffected. + +## Alternatives Considered + +1. **Keep browser storage as an offline cache.** Rejected because offline + recovery is not part of the portable-settings contract and stale caches can + override explicit backend clears. +2. **Keep retry markers without fallback reads.** Rejected because replay still + requires treating browser state as a second durable source. diff --git a/docs/decisions/INDEX.md b/docs/decisions/INDEX.md index 8ac22818cf..9025337f5c 100644 --- a/docs/decisions/INDEX.md +++ b/docs/decisions/INDEX.md @@ -33,7 +33,7 @@ Read individual ADRs for full context. Create new ones via `/record decision` or | 0025 | [Runtime cleanup uses `executors_running`](0025-runtime-cleanup-uses-executors-running.md) | accepted (amended 2026-07-06) | backend | 2026-06-22 | | 0026 | [Tauri desktop shell over native runtime](0026-tauri-desktop-shell.md) | accepted (amended 2026-07-15) | frontend, backend, cli, infra | 2026-06-23 | | 0027 | [Replayable schema migrations across SQLite and Postgres](0027-replayable-schema-migrations.md) | accepted | backend | 2026-06-24 | -| 0028 | [Backend-owned task-create last-used preferences](0028-task-create-last-used-source-of-truth.md) | accepted | backend, frontend | 2026-06-29 | +| 0028 | [Backend-owned task-create last-used preferences](0028-task-create-last-used-source-of-truth.md) | accepted (amended by 0041) | backend, frontend | 2026-06-29 | | 0029 | [Release backfill and desktop diagnostics](0029-release-backfill-and-desktop-diagnostics.md) | accepted | infra, workflow | 2026-07-01 | | 0030 | [Workspace-scoped integration settings](0030-workspace-scoped-integration-settings.md) | accepted | backend, frontend | 2026-07-01 | | 0031 | [Office skill reference files](0031-office-skill-reference-files.md) | accepted | backend | 2026-07-06 | @@ -46,3 +46,4 @@ Read individual ADRs for full context. Create new ones via `/record decision` or | 0038 | [Quick Chat Repository Isolation](0038-quick-chat-repository-isolation.md) | accepted | backend, frontend | 2026-07-14 | | 0039 | [Native desktop integration boundary](0039-native-desktop-integration-boundary.md) | accepted | desktop, frontend, backend, infra | 2026-07-15 | | 0040 | [Separate updater integrity from OS publisher identity](0040-separate-updater-integrity-from-os-publisher-identity.md) | accepted | desktop, infra, workflow | 2026-07-15 | +| 0041 | [Backend-owned portable user settings](0041-backend-owned-portable-user-settings.md) | accepted | backend, frontend | 2026-07-15 |