Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions apps/web/src/commandPaletteStore.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
32 changes: 30 additions & 2 deletions apps/web/src/commandPaletteStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -23,13 +33,30 @@ interface CommandPaletteStore {

export const useCommandPaletteStore = create<CommandPaletteStore>((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,
Expand All @@ -38,6 +65,7 @@ export const useCommandPaletteStore = create<CommandPaletteStore>((set) => ({
openThreadSearch: (request) =>
set((state) => ({
open: true,
...(state.open ? {} : { openGeneration: state.openGeneration + 1 }),
openIntent: {
kind: "search-threads",
requestId: (state.openIntent?.requestId ?? 0) + 1,
Expand Down
33 changes: 21 additions & 12 deletions apps/web/src/components/CommandPalette.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -445,13 +445,6 @@

function CommandPaletteDialog() {
const open = useCommandPaletteStore((store) => store.open);
const setOpen = useCommandPaletteStore((store) => store.setOpen);

useEffect(() => {
return () => {
setOpen(false);
};
}, [setOpen]);

if (!open) {
return null;
Expand All @@ -463,9 +456,23 @@
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(">");
Expand Down Expand Up @@ -842,13 +849,13 @@
buildProjectActionItems({
projects,
valuePrefix: "project",
icon: (project) => (
<ProjectFavicon
environmentId={project.environmentId}
cwd={project.cwd}
className={ITEM_ICON_CLASS}
/>
),

Check warning on line 858 in apps/web/src/components/CommandPalette.tsx

View workflow job for this annotation

GitHub Actions / Format, Lint, Typecheck, Test, Browser Test, Build

react(no-unstable-nested-components)

Do not define components during render.
runProject: openProjectFromSearch,
}),
[openProjectFromSearch, projects],
Expand All @@ -859,13 +866,13 @@
buildProjectActionItems({
projects,
valuePrefix: "new-thread-in",
icon: (project) => (
<ProjectFavicon
environmentId={project.environmentId}
cwd={project.cwd}
className={ITEM_ICON_CLASS}
/>
),

Check warning on line 875 in apps/web/src/components/CommandPalette.tsx

View workflow job for this annotation

GitHub Actions / Format, Lint, Typecheck, Test, Browser Test, Build

react(no-unstable-nested-components)

Do not define components during render.
runProject: async (project) => {
await startNewThreadInProjectFromContext(
{
Expand Down Expand Up @@ -896,24 +903,24 @@
...(activeThreadId ? { activeThreadId } : {}),
projectTitleById,
sortOrder: settings.sidebarThreadSortOrder,
icon: (thread) => {
const project = projectByScopedKey.get(
scopedProjectKey(scopeProjectRef(thread.environmentId, thread.projectId)),
);
if (!project) {
return <MessageSquareIcon className={ITEM_ICON_CLASS} />;
}
if (project.kind === "general-chat") {
return <MessagesSquareIcon className={ITEM_ICON_CLASS} />;
}
return (
<ProjectFavicon
environmentId={project.environmentId}
cwd={project.cwd}
className={ITEM_ICON_CLASS}
/>
);
},

Check warning on line 923 in apps/web/src/components/CommandPalette.tsx

View workflow job for this annotation

GitHub Actions / Format, Lint, Typecheck, Test, Browser Test, Build

react(no-unstable-nested-components)

Do not define components during render.
renderLeadingContent: (thread) => <ThreadRowLeadingStatus thread={thread} />,
renderTrailingContent: (thread) => <ThreadRowTrailingStatus thread={thread} />,
runThread: async (thread) => {
Expand Down Expand Up @@ -1413,7 +1420,7 @@
runtimeMode:
activeThread?.runtimeMode ?? activeDraftThread?.runtimeMode ?? DEFAULT_RUNTIME_MODE,
});
setOpen(false);
closeIfGeneration(sessionGeneration);
await navigate({
to: "/$environmentId/$threadId",
params: buildThreadRouteParams(
Expand All @@ -1428,11 +1435,12 @@
activeDraftThread?.runtimeMode,
activeThread?.runtimeMode,
codexSessionFlow,
closeIfGeneration,
currentEnvironmentProviders,
currentEnvironmentSettings,
importingProviderThreadId,
navigate,
setOpen,
sessionGeneration,
],
);

Expand Down Expand Up @@ -1691,7 +1699,7 @@
envMode: settings.defaultThreadEnvMode,
}).catch(() => undefined);
}
setOpen(false);
closeIfGeneration(sessionGeneration);
return;
}

Expand All @@ -1717,7 +1725,7 @@
await handleNewThread(createdProjectRef, {
envMode: settings.defaultThreadEnvMode,
}).catch(() => undefined);
setOpen(false);
closeIfGeneration(sessionGeneration);
} catch (error) {
toastManager.add(
stackedThreadToast({
Expand All @@ -1731,11 +1739,12 @@
[
browseEnvironmentId,
browseEnvironmentPlatform,
closeIfGeneration,
currentProjectCwdForBrowse,
handleNewThread,
navigate,
projects,
setOpen,
sessionGeneration,
settings.defaultThreadEnvMode,
settings.sidebarThreadSortOrder,
threads,
Expand Down
Loading