From 3f930f5029406353aeebabc040a7a42c0b6304fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Thu, 2 Jul 2026 13:59:57 +0100 Subject: [PATCH] fix: preserve task create defaults after refresh (#1575) --- .../settings/editors-settings-state.tsx | 2 + apps/web/components/state-provider.test.tsx | 123 ++++++++++++++++-- apps/web/components/state-provider.tsx | 36 ++--- .../hooks/use-ensure-user-settings.test.ts | 1 + apps/web/lib/ssr/user-settings.test.ts | 29 +++++ apps/web/lib/ssr/user-settings.ts | 9 ++ .../state/slices/settings/settings-slice.ts | 1 + apps/web/lib/state/slices/settings/types.ts | 1 + apps/web/lib/ws/handlers/users.test.ts | 60 +++++++++ apps/web/lib/ws/handlers/users.ts | 21 +-- 10 files changed, 247 insertions(+), 36 deletions(-) diff --git a/apps/web/components/settings/editors-settings-state.tsx b/apps/web/components/settings/editors-settings-state.tsx index 71b84b8a55..614ea98ef5 100644 --- a/apps/web/components/settings/editors-settings-state.tsx +++ b/apps/web/components/settings/editors-settings-state.tsx @@ -11,6 +11,7 @@ import { parseChangesPanelLayout, parseSystemMetricsDisplay, parseTerminalLinkBehavior, + taskCreateLastUsedHasValue, parseVoiceMode, } from "@/lib/ssr/user-settings"; import { fromApiSidebarDraft, fromApiSidebarView } from "@/lib/state/slices/ui/sidebar-view-wire"; @@ -269,6 +270,7 @@ function mapEditorSyncedLocalFields( branch: s.task_create_last_used?.branch || null, agentProfileId: s.task_create_last_used?.agent_profile_id || null, executorProfileId: s.task_create_last_used?.executor_profile_id || null, + synced: taskCreateLastUsedHasValue(s.task_create_last_used), }, jiraSavedViews: s.jira_saved_views, jiraTaskPresets: s.jira_task_presets, diff --git a/apps/web/components/state-provider.test.tsx b/apps/web/components/state-provider.test.tsx index 7dc3b6c55c..b7296974de 100644 --- a/apps/web/components/state-provider.test.tsx +++ b/apps/web/components/state-provider.test.tsx @@ -45,6 +45,47 @@ function ObserveLastUsedCache({ onSeen }: { onSeen: (value: string | null) => vo 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 }); @@ -156,16 +197,10 @@ describe("StateProvider task-create cache", () => { }); }); -describe("StateProvider task-create cache clearing", () => { - it("lets children read cached task-create choices before clearing missing backend values", async () => { +describe("StateProvider task-create cache fallback", () => { + it("keeps cached task-create choices when loaded backend settings omit them", async () => { const onSeen = vi.fn(); - window.localStorage.setItem(STORAGE_KEYS.LAST_REPOSITORY_ID, JSON.stringify("repo-cached")); - window.localStorage.setItem(STORAGE_KEYS.LAST_BRANCH, JSON.stringify("feature-cached")); - window.localStorage.setItem(STORAGE_KEYS.LAST_AGENT_PROFILE_ID, JSON.stringify("agent-cached")); - window.localStorage.setItem( - STORAGE_KEYS.LAST_EXECUTOR_PROFILE_ID, - JSON.stringify("exec-cached"), - ); + seedCachedTaskCreateChoices(); render( { , ); - expect(onSeen).toHaveBeenCalledWith(JSON.stringify("repo-cached")); + 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(() => { - expect(window.localStorage.getItem(STORAGE_KEYS.LAST_REPOSITORY_ID)).toBeNull(); + 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(); - expect(window.localStorage.getItem(STORAGE_KEYS.LAST_EXECUTOR_PROFILE_ID)).toBeNull(); }); }); }); diff --git a/apps/web/components/state-provider.tsx b/apps/web/components/state-provider.tsx index f285f80780..8ad9a1efa4 100644 --- a/apps/web/components/state-provider.tsx +++ b/apps/web/components/state-provider.tsx @@ -66,32 +66,35 @@ export function StateProvider({ children, initialState }: StoreProviderProps) { function syncTaskCreateLastUsedCache(state: AppState) { if (!state.userSettings.loaded) return; const lastUsed = state.userSettings.taskCreateLastUsed; - syncTaskCreateLastUsedCacheField(STORAGE_KEYS.LAST_REPOSITORY_ID, lastUsed?.repositoryId); - syncTaskCreateLastUsedCacheField(STORAGE_KEYS.LAST_BRANCH, lastUsed?.branch); - syncTaskCreateLastUsedCacheField(STORAGE_KEYS.LAST_AGENT_PROFILE_ID, lastUsed?.agentProfileId); + 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) { +function syncTaskCreateLastUsedCacheField( + key: string, + value: string | null | undefined, + synced: boolean | undefined, +) { if (value) { setLocalStorage(key, value); return; } - deferRemoveTaskCreateLastUsedCacheField(key); -} - -function deferRemoveTaskCreateLastUsedCacheField(key: string) { - const cachedValue = window.localStorage.getItem(key); - if (cachedValue === null) return; - window.setTimeout(() => { - if (window.localStorage.getItem(key) === cachedValue) { - removeLocalStorage(key); - } - }, 0); + if (synced) removeLocalStorage(key); } function taskCreateLastUsedEqual( @@ -102,7 +105,8 @@ function taskCreateLastUsedEqual( a?.repositoryId === b?.repositoryId && a?.branch === b?.branch && a?.agentProfileId === b?.agentProfileId && - a?.executorProfileId === b?.executorProfileId + a?.executorProfileId === b?.executorProfileId && + a?.synced === b?.synced ); } diff --git a/apps/web/hooks/use-ensure-user-settings.test.ts b/apps/web/hooks/use-ensure-user-settings.test.ts index 33d3d2761b..7aa3d80c68 100644 --- a/apps/web/hooks/use-ensure-user-settings.test.ts +++ b/apps/web/hooks/use-ensure-user-settings.test.ts @@ -171,6 +171,7 @@ describe("useEnsureUserSettings", () => { branch: "main", agentProfileId: "agent-2", executorProfileId: "exec-profile-1", + synced: true, }); }); diff --git a/apps/web/lib/ssr/user-settings.test.ts b/apps/web/lib/ssr/user-settings.test.ts index 0ff04c54e5..901f0bb325 100644 --- a/apps/web/lib/ssr/user-settings.test.ts +++ b/apps/web/lib/ssr/user-settings.test.ts @@ -59,6 +59,35 @@ describe("buildCoreFields", () => { const result = buildCoreFields(settings); expect(result.terminalFontFamily).toBeNull(); }); + + it("does not mark an empty task-create last-used object as synced", () => { + const settings = { + task_create_last_used: {}, + } as unknown as Parameters[0]; + + const result = buildCoreFields(settings); + expect(result.taskCreateLastUsed).toEqual({ + repositoryId: null, + branch: null, + agentProfileId: null, + executorProfileId: null, + synced: false, + }); + }); + + it("marks task-create last-used as synced when a field is present", () => { + const settings = { + task_create_last_used: { + repository_id: "repo-1", + }, + } as unknown as Parameters[0]; + + const result = buildCoreFields(settings); + expect(result.taskCreateLastUsed).toMatchObject({ + repositoryId: "repo-1", + synced: true, + }); + }); }); describe("buildTerminalFields via buildCoreFields", () => { diff --git a/apps/web/lib/ssr/user-settings.ts b/apps/web/lib/ssr/user-settings.ts index 9e330e13b2..70bcfeec55 100644 --- a/apps/web/lib/ssr/user-settings.ts +++ b/apps/web/lib/ssr/user-settings.ts @@ -64,12 +64,21 @@ function parseSidebarTaskPrefs(value: SidebarTaskPrefsApi | undefined) { }; } +export function taskCreateLastUsedHasValue( + value: UserSettingsData["task_create_last_used"] | undefined, +) { + return Boolean( + value?.repository_id || value?.branch || value?.agent_profile_id || value?.executor_profile_id, + ); +} + function parseTaskCreateLastUsed(value: UserSettingsData["task_create_last_used"] | undefined) { return { repositoryId: value?.repository_id || null, branch: value?.branch || null, agentProfileId: value?.agent_profile_id || null, executorProfileId: value?.executor_profile_id || null, + synced: taskCreateLastUsedHasValue(value), }; } diff --git a/apps/web/lib/state/slices/settings/settings-slice.ts b/apps/web/lib/state/slices/settings/settings-slice.ts index cd8a0225ca..1d5b6d63c5 100644 --- a/apps/web/lib/state/slices/settings/settings-slice.ts +++ b/apps/web/lib/state/slices/settings/settings-slice.ts @@ -47,6 +47,7 @@ export const defaultSettingsState: SettingsSliceState = { branch: null, agentProfileId: null, executorProfileId: null, + synced: false, }, jiraSavedViews: undefined, jiraTaskPresets: undefined, diff --git a/apps/web/lib/state/slices/settings/types.ts b/apps/web/lib/state/slices/settings/types.ts index 1c9d49e44c..38bd4abcfe 100644 --- a/apps/web/lib/state/slices/settings/types.ts +++ b/apps/web/lib/state/slices/settings/types.ts @@ -181,6 +181,7 @@ export type TaskCreateLastUsedState = { branch: string | null; agentProfileId: string | null; executorProfileId: string | null; + synced?: boolean; }; export type VoiceModeState = { diff --git a/apps/web/lib/ws/handlers/users.test.ts b/apps/web/lib/ws/handlers/users.test.ts index de13d9d186..b1994f6558 100644 --- a/apps/web/lib/ws/handlers/users.test.ts +++ b/apps/web/lib/ws/handlers/users.test.ts @@ -91,6 +91,66 @@ describe("user settings websocket handler", () => { }); }); +describe("user settings websocket task-create last-used", () => { + it("does not mark empty task-create last-used broadcasts as synced", () => { + const store = makeStore(); + + registerUsersHandlers(store)["user.settings.updated"]?.( + userSettingsMessage({ task_create_last_used: {} }), + ); + + expect(store.getState().userSettings.taskCreateLastUsed).toEqual({ + repositoryId: null, + branch: null, + agentProfileId: null, + executorProfileId: null, + synced: false, + }); + }); + + it("marks task-create last-used broadcasts as synced when a field is present", () => { + const store = makeStore(); + + registerUsersHandlers(store)["user.settings.updated"]?.( + userSettingsMessage({ task_create_last_used: { repository_id: "repo-1" } }), + ); + + expect(store.getState().userSettings.taskCreateLastUsed).toMatchObject({ + repositoryId: "repo-1", + synced: true, + }); + }); + + it("preserves task-create last-used state when broadcasts omit it", () => { + const store = makeStore(); + store.setState((state) => ({ + ...state, + userSettings: { + ...state.userSettings, + taskCreateLastUsed: { + repositoryId: "repo-1", + branch: "main", + agentProfileId: "agent-1", + executorProfileId: "exec-1", + synced: true, + }, + }, + })); + + registerUsersHandlers(store)["user.settings.updated"]?.( + userSettingsMessage({ keyboard_shortcuts: {} }), + ); + + expect(store.getState().userSettings.taskCreateLastUsed).toEqual({ + repositoryId: "repo-1", + branch: "main", + agentProfileId: "agent-1", + executorProfileId: "exec-1", + synced: true, + }); + }); +}); + describe("user settings websocket sidebar settings", () => { it("preserves userSettings sidebar fields when payload omits them", () => { const store = makeStore(); diff --git a/apps/web/lib/ws/handlers/users.ts b/apps/web/lib/ws/handlers/users.ts index 709ed9af16..b118998d83 100644 --- a/apps/web/lib/ws/handlers/users.ts +++ b/apps/web/lib/ws/handlers/users.ts @@ -5,6 +5,7 @@ import type { WsHandlers } from "@/lib/ws/handlers/types"; import { parseChangesPanelLayout, parseSystemMetricsDisplay, + taskCreateLastUsedHasValue, parseVoiceMode, } from "@/lib/ssr/user-settings"; import { fromApiSidebarDraft, fromApiSidebarView } from "@/lib/state/slices/ui/sidebar-view-wire"; @@ -29,7 +30,7 @@ function buildUserSettingsState(state: AppState, payload: UserSettingsUpdatedPay ...buildBehaviorSettings(payload), ...buildSidebarSettings(state, payload), ...buildLspSettings(payload), - ...buildSyncedLocalSettings(payload), + ...buildSyncedLocalSettings(state, payload), defaultUtilityAgentId: payload.default_utility_agent_id || null, keyboardShortcuts: payload.keyboard_shortcuts ?? {}, changesPanelLayout: parseChangesPanelLayout(payload.changes_panel_layout), @@ -46,15 +47,19 @@ function buildLspSettings(payload: UserSettingsUpdatedPayload) { }; } -function buildSyncedLocalSettings(payload: UserSettingsUpdatedPayload) { +function buildSyncedLocalSettings(state: AppState, payload: UserSettingsUpdatedPayload) { return { savedLayouts: payload.saved_layouts ?? [], - taskCreateLastUsed: { - repositoryId: payload.task_create_last_used?.repository_id || null, - branch: payload.task_create_last_used?.branch || null, - agentProfileId: payload.task_create_last_used?.agent_profile_id || null, - executorProfileId: payload.task_create_last_used?.executor_profile_id || null, - }, + taskCreateLastUsed: + payload.task_create_last_used === undefined + ? state.userSettings.taskCreateLastUsed + : { + repositoryId: payload.task_create_last_used?.repository_id || null, + branch: payload.task_create_last_used?.branch || null, + agentProfileId: payload.task_create_last_used?.agent_profile_id || null, + executorProfileId: payload.task_create_last_used?.executor_profile_id || null, + synced: taskCreateLastUsedHasValue(payload.task_create_last_used), + }, jiraSavedViews: payload.jira_saved_views, jiraTaskPresets: payload.jira_task_presets, githubSavedPresets: payload.github_saved_presets,