diff --git a/apps/web/src/commandPaletteStore.test.ts b/apps/web/src/commandPaletteStore.test.ts new file mode 100644 index 000000000..917217bbc --- /dev/null +++ b/apps/web/src/commandPaletteStore.test.ts @@ -0,0 +1,57 @@ +import { beforeEach, describe, expect, it } from "vite-plus/test"; + +import { useCommandPaletteStore } from "./commandPaletteStore"; + +function resetStore(): void { + useCommandPaletteStore.setState({ open: false, openGeneration: 0, openIntent: null }); +} + +describe("command palette store", () => { + beforeEach(resetStore); + + it("advances the open generation once per opening, not per open call", () => { + const { setOpen, openAddProject } = useCommandPaletteStore.getState(); + + setOpen(true); + expect(useCommandPaletteStore.getState().openGeneration).toBe(1); + + // Re-purposing an already-open palette stays in the same session. + setOpen(true); + openAddProject(); + expect(useCommandPaletteStore.getState().openGeneration).toBe(1); + + setOpen(false); + setOpen(true); + expect(useCommandPaletteStore.getState().openGeneration).toBe(2); + }); + + it("closes via closeIfGeneration only for the current session", () => { + const { setOpen, closeIfGeneration } = useCommandPaletteStore.getState(); + + setOpen(true); + const firstSession = useCommandPaletteStore.getState().openGeneration; + setOpen(false); + setOpen(true); + + // A continuation from the first session must not close the second. + closeIfGeneration(firstSession); + expect(useCommandPaletteStore.getState().open).toBe(true); + + closeIfGeneration(useCommandPaletteStore.getState().openGeneration); + expect(useCommandPaletteStore.getState().open).toBe(false); + + // Already closed: a repeat stale close stays a no-op. + closeIfGeneration(firstSession); + expect(useCommandPaletteStore.getState().open).toBe(false); + }); + + it("clears the open intent when a guarded close lands", () => { + const { openAddProject, closeIfGeneration } = useCommandPaletteStore.getState(); + + openAddProject(); + expect(useCommandPaletteStore.getState().openIntent?.kind).toBe("add-project"); + + closeIfGeneration(useCommandPaletteStore.getState().openGeneration); + expect(useCommandPaletteStore.getState().openIntent).toBeNull(); + }); +}); diff --git a/apps/web/src/commandPaletteStore.ts b/apps/web/src/commandPaletteStore.ts index 9bff87b53..5d07a961b 100644 --- a/apps/web/src/commandPaletteStore.ts +++ b/apps/web/src/commandPaletteStore.ts @@ -13,8 +13,18 @@ type CommandPaletteOpenIntent = interface CommandPaletteStore { open: boolean; + /** + * Increments on every opening. Async flows that close the palette after an + * await must capture this when they start and close via `closeIfGeneration`, + * so a continuation that outlives its own palette session (the user closed + * or reopened it while the request was in flight) cannot slam a palette it + * does not own. + */ + openGeneration: number; openIntent: CommandPaletteOpenIntent | null; setOpen: (open: boolean) => void; + /** Closes only if the palette is still on the given open generation. */ + closeIfGeneration: (generation: number) => void; toggleOpen: () => void; openAddProject: () => void; openThreadSearch: (request: CommandPaletteThreadSearchRequest) => void; @@ -23,13 +33,30 @@ interface CommandPaletteStore { export const useCommandPaletteStore = create((set) => ({ open: false, + openGeneration: 0, openIntent: null, - setOpen: (open) => set({ open, ...(open ? {} : { openIntent: null }) }), + setOpen: (open) => + set((state) => ({ + open, + ...(open + ? state.open + ? {} + : { openGeneration: state.openGeneration + 1 } + : { openIntent: null }), + })), + closeIfGeneration: (generation) => + set((state) => + state.open && state.openGeneration === generation ? { open: false, openIntent: null } : state, + ), toggleOpen: () => - set((state) => ({ open: !state.open, ...(state.open ? { openIntent: null } : {}) })), + set((state) => ({ + open: !state.open, + ...(state.open ? { openIntent: null } : { openGeneration: state.openGeneration + 1 }), + })), openAddProject: () => set((state) => ({ open: true, + ...(state.open ? {} : { openGeneration: state.openGeneration + 1 }), openIntent: { kind: "add-project", requestId: (state.openIntent?.requestId ?? 0) + 1, @@ -38,6 +65,7 @@ export const useCommandPaletteStore = create((set) => ({ openThreadSearch: (request) => set((state) => ({ open: true, + ...(state.open ? {} : { openGeneration: state.openGeneration + 1 }), openIntent: { kind: "search-threads", requestId: (state.openIntent?.requestId ?? 0) + 1, diff --git a/apps/web/src/components/CommandPalette.tsx b/apps/web/src/components/CommandPalette.tsx index 5fefbbc97..ef4d1a56b 100644 --- a/apps/web/src/components/CommandPalette.tsx +++ b/apps/web/src/components/CommandPalette.tsx @@ -445,13 +445,6 @@ export function CommandPalette({ children }: { children: ReactNode }) { function CommandPaletteDialog() { const open = useCommandPaletteStore((store) => store.open); - const setOpen = useCommandPaletteStore((store) => store.setOpen); - - useEffect(() => { - return () => { - setOpen(false); - }; - }, [setOpen]); if (!open) { return null; @@ -463,9 +456,23 @@ function CommandPaletteDialog() { function OpenCommandPaletteDialog() { const navigate = useNavigate(); const setOpen = useCommandPaletteStore((store) => store.setOpen); + const closeIfGeneration = useCommandPaletteStore((store) => store.closeIfGeneration); const openIntent = useCommandPaletteStore((store) => store.openIntent); const clearOpenIntent = useCommandPaletteStore((store) => store.clearOpenIntent); const composerHandleRef = useComposerHandleContext(); + // This component mounts once per palette session, so its mount-time + // generation identifies the session every deferred close below belongs to. + // Async flows and the unmount reset close via `closeIfGeneration`: if the + // user closed or reopened the palette while a request was in flight (or + // React deferred the unmount cleanup past a new session), the stale close + // is a no-op instead of slamming a palette it does not own. + const [sessionGeneration] = useState(() => useCommandPaletteStore.getState().openGeneration); + + useEffect(() => { + return () => { + closeIfGeneration(sessionGeneration); + }; + }, [closeIfGeneration, sessionGeneration]); const [query, setQuery] = useState(""); const deferredQuery = useDeferredValue(query); const isActionsOnly = deferredQuery.startsWith(">"); @@ -1413,7 +1420,7 @@ function OpenCommandPaletteDialog() { runtimeMode: activeThread?.runtimeMode ?? activeDraftThread?.runtimeMode ?? DEFAULT_RUNTIME_MODE, }); - setOpen(false); + closeIfGeneration(sessionGeneration); await navigate({ to: "/$environmentId/$threadId", params: buildThreadRouteParams( @@ -1428,11 +1435,12 @@ function OpenCommandPaletteDialog() { activeDraftThread?.runtimeMode, activeThread?.runtimeMode, codexSessionFlow, + closeIfGeneration, currentEnvironmentProviders, currentEnvironmentSettings, importingProviderThreadId, navigate, - setOpen, + sessionGeneration, ], ); @@ -1691,7 +1699,7 @@ function OpenCommandPaletteDialog() { envMode: settings.defaultThreadEnvMode, }).catch(() => undefined); } - setOpen(false); + closeIfGeneration(sessionGeneration); return; } @@ -1717,7 +1725,7 @@ function OpenCommandPaletteDialog() { await handleNewThread(createdProjectRef, { envMode: settings.defaultThreadEnvMode, }).catch(() => undefined); - setOpen(false); + closeIfGeneration(sessionGeneration); } catch (error) { toastManager.add( stackedThreadToast({ @@ -1731,11 +1739,12 @@ function OpenCommandPaletteDialog() { [ browseEnvironmentId, browseEnvironmentPlatform, + closeIfGeneration, currentProjectCwdForBrowse, handleNewThread, navigate, projects, - setOpen, + sessionGeneration, settings.defaultThreadEnvMode, settings.sidebarThreadSortOrder, threads,