mirror of
https://github.com/aaif-goose/goose.git
synced 2026-07-03 14:10:03 +02:00
chore: goose2 UI state refactor (part 1) (#9049)
This commit is contained in:
+100
-75
@@ -1,4 +1,6 @@
|
||||
import { useCallback, useEffect, useRef, useState } from "react";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import { toast } from "sonner";
|
||||
import { Sidebar } from "@/features/sidebar/ui/Sidebar";
|
||||
import { CreateProjectDialog } from "@/features/projects/ui/CreateProjectDialog";
|
||||
import { archiveProject } from "@/features/projects/api/projects";
|
||||
@@ -8,12 +10,21 @@ import type { SectionId } from "@/features/settings/ui/SettingsModal";
|
||||
import { OPEN_SETTINGS_EVENT } from "@/features/settings/lib/settingsEvents";
|
||||
import { TopBar } from "./ui/TopBar";
|
||||
import { useChatStore } from "@/features/chat/stores/chatStore";
|
||||
import { selectMessagesBySession } from "@/features/chat/stores/chatSelectors";
|
||||
import {
|
||||
type ChatSession,
|
||||
useChatSessionStore,
|
||||
} from "@/features/chat/stores/chatSessionStore";
|
||||
import {
|
||||
selectActiveSessionId,
|
||||
selectHasHydratedSessions,
|
||||
selectSessions,
|
||||
selectSessionsLoading,
|
||||
} from "@/features/chat/stores/chatSessionSelectors";
|
||||
import { useAgentStore } from "@/features/agents/stores/agentStore";
|
||||
import { selectSelectedProvider } from "@/features/agents/stores/agentSelectors";
|
||||
import { useProjectStore } from "@/features/projects/stores/projectStore";
|
||||
import { selectProjects } from "@/features/projects/stores/projectSelectors";
|
||||
import { findExistingDraft } from "@/features/chat/lib/newChat";
|
||||
import { DEFAULT_CHAT_TITLE } from "@/features/chat/lib/sessionTitle";
|
||||
import { useAppStartup } from "./hooks/useAppStartup";
|
||||
@@ -23,6 +34,10 @@ import { resolveSupportedSessionModelPreference } from "./lib/resolveSupportedSe
|
||||
import { useCreatePersonaNavigation } from "./hooks/useCreatePersonaNavigation";
|
||||
import { AppShellContent } from "./ui/AppShellContent";
|
||||
import { acpPrepareSession, acpSetModel } from "@/shared/api/acp";
|
||||
import {
|
||||
updateSessionProject,
|
||||
updateSessionTitle,
|
||||
} from "@/features/chat/stores/chatSessionOperations";
|
||||
import {
|
||||
clearReplayBuffer,
|
||||
getAndDeleteReplayBuffer,
|
||||
@@ -103,6 +118,7 @@ async function syncWindowMinimumSize() {
|
||||
}
|
||||
|
||||
export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
const { t } = useTranslation("chat");
|
||||
const [sidebarCollapsed, setSidebarCollapsed] = useState(false);
|
||||
const [sidebarWidth, setSidebarWidth] = useState(SIDEBAR_DEFAULT_WIDTH);
|
||||
const [isResizing, setIsResizing] = useState(false);
|
||||
@@ -120,10 +136,21 @@ export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
loadStoredHomeSessionId(),
|
||||
);
|
||||
|
||||
const chatStore = useChatStore();
|
||||
const sessionStore = useChatSessionStore();
|
||||
const agentStore = useAgentStore();
|
||||
const projectStore = useProjectStore();
|
||||
const messagesBySession = useChatStore(selectMessagesBySession);
|
||||
const setChatActiveSession = useChatStore((s) => s.setActiveSession);
|
||||
const cleanupChatSession = useChatStore((s) => s.cleanupSession);
|
||||
const sessions = useChatSessionStore(selectSessions);
|
||||
const activeSessionId = useChatSessionStore(selectActiveSessionId);
|
||||
const hasHydratedSessions = useChatSessionStore(selectHasHydratedSessions);
|
||||
const sessionsLoading = useChatSessionStore(selectSessionsLoading);
|
||||
const createSession = useChatSessionStore((s) => s.createSession);
|
||||
const patchSession = useChatSessionStore((s) => s.patchSession);
|
||||
const setActiveSession = useChatSessionStore((s) => s.setActiveSession);
|
||||
const archiveSession = useChatSessionStore((s) => s.archiveSession);
|
||||
const selectedProvider = useAgentStore(selectSelectedProvider);
|
||||
const projects = useProjectStore(selectProjects);
|
||||
const fetchProjects = useProjectStore((s) => s.fetchProjects);
|
||||
const reorderProjects = useProjectStore((s) => s.reorderProjects);
|
||||
const providerInventoryEntries = useProviderInventoryStore((s) => s.entries);
|
||||
const startup = useAppStartup();
|
||||
const onboardingGate = useOnboardingGate(startup.ready);
|
||||
@@ -182,10 +209,8 @@ export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
}, []);
|
||||
|
||||
useEffect(() => {
|
||||
projectStore.fetchProjects();
|
||||
}, [projectStore.fetchProjects]);
|
||||
|
||||
const { activeSessionId } = sessionStore;
|
||||
fetchProjects();
|
||||
}, [fetchProjects]);
|
||||
|
||||
useEffect(() => {
|
||||
if (activeView === "chat" && activeSessionId) {
|
||||
@@ -194,23 +219,23 @@ export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
}, [activeSessionId, activeView]);
|
||||
|
||||
const activeSession = activeSessionId
|
||||
? sessionStore.getSession(activeSessionId)
|
||||
? sessions.find((session) => session.id === activeSessionId)
|
||||
: undefined;
|
||||
const homeSession = homeSessionId
|
||||
? sessionStore.getSession(homeSessionId)
|
||||
? sessions.find((session) => session.id === homeSessionId)
|
||||
: undefined;
|
||||
|
||||
useHomeSessionStateSync({
|
||||
homeSessionId,
|
||||
homeSession,
|
||||
messagesBySession: chatStore.messagesBySession,
|
||||
hasHydratedSessions: sessionStore.hasHydratedSessions,
|
||||
isLoading: sessionStore.isLoading,
|
||||
messagesBySession,
|
||||
hasHydratedSessions,
|
||||
isLoading: sessionsLoading,
|
||||
setHomeSessionId,
|
||||
});
|
||||
|
||||
const ensureHomeSession = useCallback(async () => {
|
||||
if (!sessionStore.hasHydratedSessions || sessionStore.isLoading) {
|
||||
if (!hasHydratedSessions || sessionsLoading) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -226,11 +251,11 @@ export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
) {
|
||||
const sessionModelPreference =
|
||||
await resolveSupportedSessionModelPreference(
|
||||
agentStore.selectedProvider ?? "goose",
|
||||
selectedProvider ?? "goose",
|
||||
providerInventoryEntries,
|
||||
);
|
||||
const project = homeSession.projectId
|
||||
? (projectStore.projects.find(
|
||||
? (projects.find(
|
||||
(candidate) => candidate.id === homeSession.projectId,
|
||||
) ?? null)
|
||||
: null;
|
||||
@@ -243,14 +268,14 @@ export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
const shouldClearHomeModel =
|
||||
sessionModelPreference.providerId !== homeSession.providerId ||
|
||||
!sessionModelPreference.modelId;
|
||||
sessionStore.updateSession(homeSession.id, {
|
||||
patchSession(homeSession.id, {
|
||||
providerId: sessionModelPreference.providerId,
|
||||
modelId: shouldClearHomeModel ? undefined : homeSession.modelId,
|
||||
modelName: shouldClearHomeModel ? undefined : homeSession.modelName,
|
||||
});
|
||||
if (sessionModelPreference.modelId) {
|
||||
await acpSetModel(homeSession.id, sessionModelPreference.modelId);
|
||||
sessionStore.updateSession(homeSession.id, {
|
||||
patchSession(homeSession.id, {
|
||||
modelId: sessionModelPreference.modelId,
|
||||
modelName: sessionModelPreference.modelName,
|
||||
});
|
||||
@@ -261,10 +286,10 @@ export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
const workingDir = await resolveSessionCwd(null);
|
||||
const sessionModelPreference =
|
||||
await resolveSupportedSessionModelPreference(
|
||||
agentStore.selectedProvider ?? "goose",
|
||||
selectedProvider ?? "goose",
|
||||
providerInventoryEntries,
|
||||
);
|
||||
const session = await sessionStore.createSession({
|
||||
const session = await createSession({
|
||||
title: DEFAULT_CHAT_TITLE,
|
||||
providerId: sessionModelPreference.providerId,
|
||||
workingDir,
|
||||
@@ -284,13 +309,14 @@ export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
}
|
||||
}
|
||||
}, [
|
||||
agentStore.selectedProvider,
|
||||
selectedProvider,
|
||||
createSession,
|
||||
hasHydratedSessions,
|
||||
homeSession,
|
||||
providerInventoryEntries,
|
||||
projectStore.projects,
|
||||
sessionStore.hasHydratedSessions,
|
||||
sessionStore,
|
||||
sessionStore.isLoading,
|
||||
projects,
|
||||
sessionsLoading,
|
||||
patchSession,
|
||||
]);
|
||||
|
||||
useEffect(() => {
|
||||
@@ -309,7 +335,7 @@ export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
`[perf:newtab] createNewTab start (project=${project?.id ?? "none"})`,
|
||||
);
|
||||
const providerId =
|
||||
project?.preferredProvider ?? agentStore.selectedProvider ?? "goose";
|
||||
project?.preferredProvider ?? selectedProvider ?? "goose";
|
||||
const sessionModelPreference =
|
||||
await resolveSupportedSessionModelPreference(
|
||||
providerId,
|
||||
@@ -330,9 +356,9 @@ export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
});
|
||||
|
||||
if (existingDraft) {
|
||||
sessionStore.setActiveSession(existingDraft.id);
|
||||
setActiveSession(existingDraft.id);
|
||||
setActiveView("chat");
|
||||
chatStore.setActiveSession(existingDraft.id);
|
||||
setChatActiveSession(existingDraft.id);
|
||||
perfLog(
|
||||
`[perf:newtab] ${existingDraft.id.slice(0, 8)} reused draft in ${(performance.now() - tStart).toFixed(1)}ms`,
|
||||
);
|
||||
@@ -340,7 +366,7 @@ export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
}
|
||||
|
||||
const workingDir = await resolveSessionCwd(project);
|
||||
const session = await sessionStore.createSession({
|
||||
const session = await createSession({
|
||||
title,
|
||||
projectId: project?.id,
|
||||
providerId: sessionModelPreference.providerId,
|
||||
@@ -348,19 +374,20 @@ export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
modelId: sessionModelPreference.modelId,
|
||||
modelName: sessionModelPreference.modelName,
|
||||
});
|
||||
sessionStore.setActiveSession(session.id);
|
||||
setActiveSession(session.id);
|
||||
setActiveView("chat");
|
||||
chatStore.setActiveSession(session.id);
|
||||
setChatActiveSession(session.id);
|
||||
perfLog(
|
||||
`[perf:newtab] ${session.id.slice(0, 8)} created session in ${(performance.now() - tStart).toFixed(1)}ms`,
|
||||
);
|
||||
return session;
|
||||
},
|
||||
[
|
||||
agentStore.selectedProvider,
|
||||
chatStore,
|
||||
selectedProvider,
|
||||
createSession,
|
||||
providerInventoryEntries,
|
||||
sessionStore,
|
||||
setActiveSession,
|
||||
setChatActiveSession,
|
||||
],
|
||||
);
|
||||
|
||||
@@ -374,7 +401,7 @@ export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
const handleStartChatWithSkill = useCallback(
|
||||
(skill: SkillInfo, projectId?: string | null) => {
|
||||
const project = projectId
|
||||
? projectStore.projects.find((candidate) => candidate.id === projectId)
|
||||
? projects.find((candidate) => candidate.id === projectId)
|
||||
: undefined;
|
||||
|
||||
void createNewTab(DEFAULT_CHAT_TITLE, project)
|
||||
@@ -387,38 +414,38 @@ export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
console.error("Failed to start chat with skill:", error);
|
||||
});
|
||||
},
|
||||
[createNewTab, projectStore.projects],
|
||||
[createNewTab, projects],
|
||||
);
|
||||
|
||||
const handleNewChatInProject = useCallback(
|
||||
(projectId: string) => {
|
||||
const project = projectStore.projects.find((p) => p.id === projectId);
|
||||
const project = projects.find((p) => p.id === projectId);
|
||||
if (project) {
|
||||
void createNewTab(DEFAULT_CHAT_TITLE, project);
|
||||
}
|
||||
},
|
||||
[createNewTab, projectStore.projects],
|
||||
[createNewTab, projects],
|
||||
);
|
||||
|
||||
const handleArchiveProject = useCallback(
|
||||
async (projectId: string) => {
|
||||
try {
|
||||
await archiveProject(projectId);
|
||||
projectStore.fetchProjects();
|
||||
fetchProjects();
|
||||
} catch {
|
||||
// best-effort
|
||||
}
|
||||
},
|
||||
[projectStore.fetchProjects],
|
||||
[fetchProjects],
|
||||
);
|
||||
|
||||
const clearActiveSession = useCallback(
|
||||
(sessionId: string) => {
|
||||
chatStore.cleanupSession(sessionId);
|
||||
sessionStore.setActiveSession(null);
|
||||
cleanupChatSession(sessionId);
|
||||
setActiveSession(null);
|
||||
setActiveView("home");
|
||||
},
|
||||
[chatStore, sessionStore],
|
||||
[cleanupChatSession, setActiveSession],
|
||||
);
|
||||
const openSettings = useCallback((section: SectionId = "appearance") => {
|
||||
setSettingsInitialSection(section);
|
||||
@@ -456,43 +483,43 @@ export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
const wasActiveSession = currentActiveSessionId === sessionId;
|
||||
|
||||
try {
|
||||
await sessionStore.archiveSession(sessionId);
|
||||
chatStore.cleanupSession(sessionId);
|
||||
await archiveSession(sessionId);
|
||||
cleanupChatSession(sessionId);
|
||||
|
||||
if (!wasActiveSession) {
|
||||
return;
|
||||
}
|
||||
|
||||
sessionStore.setActiveSession(null);
|
||||
setActiveSession(null);
|
||||
setActiveView("home");
|
||||
} catch {
|
||||
// best-effort
|
||||
}
|
||||
},
|
||||
[chatStore, sessionStore],
|
||||
[archiveSession, cleanupChatSession, setActiveSession],
|
||||
);
|
||||
|
||||
const handleEditProject = useCallback(
|
||||
(projectId: string) => {
|
||||
const project = projectStore.projects.find((p) => p.id === projectId);
|
||||
const project = projects.find((p) => p.id === projectId);
|
||||
if (project) {
|
||||
setEditingProject(project);
|
||||
setCreateProjectOpen(true);
|
||||
}
|
||||
},
|
||||
[projectStore.projects],
|
||||
[projects],
|
||||
);
|
||||
|
||||
const handleMoveToProject = useCallback(
|
||||
(sessionId: string, projectId: string | null) => {
|
||||
sessionStore.updateSession(sessionId, { projectId });
|
||||
|
||||
const session = useChatSessionStore.getState().getSession(sessionId);
|
||||
if (!session) {
|
||||
return;
|
||||
}
|
||||
|
||||
void (async () => {
|
||||
await updateSessionProject(sessionId, projectId);
|
||||
|
||||
const nextProject =
|
||||
projectId == null
|
||||
? null
|
||||
@@ -505,27 +532,25 @@ export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
}
|
||||
await acpPrepareSession(
|
||||
sessionId,
|
||||
session.providerId ?? agentStore.selectedProvider ?? "goose",
|
||||
session.providerId ?? selectedProvider ?? "goose",
|
||||
workingDir,
|
||||
);
|
||||
})().catch((error) => {
|
||||
console.error(
|
||||
"Failed to update ACP session project working directory:",
|
||||
error,
|
||||
);
|
||||
console.error("Failed to move chat to project:", error);
|
||||
toast.error(t("notifications.moveError"));
|
||||
});
|
||||
},
|
||||
[agentStore.selectedProvider, sessionStore],
|
||||
[selectedProvider, t],
|
||||
);
|
||||
|
||||
const handleRenameChat = useCallback(
|
||||
(sessionId: string, nextTitle: string) => {
|
||||
sessionStore.updateSession(sessionId, {
|
||||
title: nextTitle,
|
||||
userSetName: true,
|
||||
void updateSessionTitle(sessionId, nextTitle).catch((error) => {
|
||||
console.error("Failed to rename session:", error);
|
||||
toast.error(t("notifications.renameError"));
|
||||
});
|
||||
},
|
||||
[sessionStore],
|
||||
[t],
|
||||
);
|
||||
|
||||
const openCreateProjectDialog = useCallback(
|
||||
@@ -546,23 +571,23 @@ export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
if (homeSessionId === sessionId) {
|
||||
setHomeSessionId(null);
|
||||
}
|
||||
sessionStore.setActiveSession(sessionId);
|
||||
setActiveSession(sessionId);
|
||||
setActiveView("chat");
|
||||
chatStore.setActiveSession(sessionId);
|
||||
setChatActiveSession(sessionId);
|
||||
useChatStore.getState().markSessionRead(sessionId);
|
||||
},
|
||||
[chatStore, homeSessionId, sessionStore],
|
||||
[homeSessionId, setActiveSession, setChatActiveSession],
|
||||
);
|
||||
|
||||
const handleSelectSession = useCallback(
|
||||
(id: string) => {
|
||||
sessionStore.setActiveSession(id);
|
||||
setActiveSession(id);
|
||||
setActiveView("chat");
|
||||
chatStore.setActiveSession(id);
|
||||
setChatActiveSession(id);
|
||||
useChatStore.getState().markSessionRead(id);
|
||||
loadSessionMessages(id);
|
||||
},
|
||||
[sessionStore, chatStore, loadSessionMessages],
|
||||
[setActiveSession, setChatActiveSession, loadSessionMessages],
|
||||
);
|
||||
|
||||
const handleSelectSearchResult = useCallback(
|
||||
@@ -580,11 +605,11 @@ export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
const handleNavigate = useCallback(
|
||||
(view: AppView) => {
|
||||
if (view !== "chat") {
|
||||
sessionStore.setActiveSession(null);
|
||||
setActiveSession(null);
|
||||
}
|
||||
setActiveView(view);
|
||||
},
|
||||
[sessionStore],
|
||||
[setActiveSession],
|
||||
);
|
||||
|
||||
const handleCreatePersona = useCreatePersonaNavigation(() =>
|
||||
@@ -717,13 +742,13 @@ export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
// Cmd+N opens new conversation screen
|
||||
if (e.key === "n" && e.metaKey) {
|
||||
e.preventDefault();
|
||||
sessionStore.setActiveSession(null);
|
||||
setActiveSession(null);
|
||||
setActiveView("home");
|
||||
}
|
||||
};
|
||||
window.addEventListener("keydown", handler);
|
||||
return () => window.removeEventListener("keydown", handler);
|
||||
}, [clearActiveSession, sessionStore, toggleSidebar]);
|
||||
}, [clearActiveSession, setActiveSession, toggleSidebar]);
|
||||
|
||||
if (!startup.ready) {
|
||||
return (
|
||||
@@ -768,7 +793,7 @@ export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
onNavigate={handleNavigate}
|
||||
onNewChatInProject={handleNewChatInProject}
|
||||
onNewChat={() => {
|
||||
sessionStore.setActiveSession(null);
|
||||
setActiveSession(null);
|
||||
setActiveView("home");
|
||||
}}
|
||||
onCreateProject={() => openCreateProjectDialog()}
|
||||
@@ -777,12 +802,12 @@ export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
onArchiveChat={handleArchiveChat}
|
||||
onRenameChat={handleRenameChat}
|
||||
onMoveToProject={handleMoveToProject}
|
||||
onReorderProject={projectStore.reorderProjects}
|
||||
onReorderProject={reorderProjects}
|
||||
onSelectSession={handleSelectSession}
|
||||
onSelectSearchResult={handleSelectSearchResult}
|
||||
activeView={activeView}
|
||||
activeSessionId={activeSessionId}
|
||||
projects={projectStore.projects}
|
||||
projects={projects}
|
||||
className="h-full rounded-xl"
|
||||
/>
|
||||
</div>
|
||||
@@ -832,7 +857,7 @@ export function AppShell({ children }: { children?: React.ReactNode }) {
|
||||
pendingProjectCreatedRef.current = null;
|
||||
}}
|
||||
onCreated={(project) => {
|
||||
projectStore.fetchProjects();
|
||||
fetchProjects();
|
||||
pendingProjectCreatedRef.current?.(project.id);
|
||||
pendingProjectCreatedRef.current = null;
|
||||
setCreateProjectInitialWorkingDir(null);
|
||||
|
||||
@@ -1,5 +1,9 @@
|
||||
import { useEffect, useCallback, useRef } from "react";
|
||||
import { useAgentStore } from "../stores/agentStore";
|
||||
import {
|
||||
selectPersonas,
|
||||
selectPersonasLoading,
|
||||
} from "../stores/agentSelectors";
|
||||
import type {
|
||||
CreatePersonaRequest,
|
||||
UpdatePersonaRequest,
|
||||
@@ -9,32 +13,36 @@ import * as api from "@/shared/api/agents";
|
||||
const REFRESH_INTERVAL_MS = 60_000;
|
||||
|
||||
export function usePersonas() {
|
||||
const store = useAgentStore();
|
||||
const personas = useAgentStore(selectPersonas);
|
||||
const personasLoading = useAgentStore(selectPersonasLoading);
|
||||
const setPersonas = useAgentStore((s) => s.setPersonas);
|
||||
const addPersona = useAgentStore((s) => s.addPersona);
|
||||
const updatePersonaInStore = useAgentStore((s) => s.updatePersona);
|
||||
const removePersona = useAgentStore((s) => s.removePersona);
|
||||
const setPersonasLoading = useAgentStore((s) => s.setPersonasLoading);
|
||||
const refreshTimerRef = useRef<ReturnType<typeof setInterval> | null>(null);
|
||||
|
||||
// biome-ignore lint/correctness/useExhaustiveDependencies: store is stable and should not trigger re-creation
|
||||
const loadPersonas = useCallback(async () => {
|
||||
store.setPersonasLoading(true);
|
||||
setPersonasLoading(true);
|
||||
try {
|
||||
const personas = await api.listPersonas();
|
||||
store.setPersonas(personas);
|
||||
setPersonas(personas);
|
||||
} catch (error) {
|
||||
console.error("Failed to load personas:", error);
|
||||
// Fall back to empty list - builtins will come from backend
|
||||
} finally {
|
||||
store.setPersonasLoading(false);
|
||||
setPersonasLoading(false);
|
||||
}
|
||||
}, []);
|
||||
}, [setPersonas, setPersonasLoading]);
|
||||
|
||||
// biome-ignore lint/correctness/useExhaustiveDependencies: store is stable and should not trigger re-creation
|
||||
const refreshFromDisk = useCallback(async () => {
|
||||
try {
|
||||
const personas = await api.refreshPersonas();
|
||||
store.setPersonas(personas);
|
||||
setPersonas(personas);
|
||||
} catch (error) {
|
||||
console.error("Failed to refresh personas from disk:", error);
|
||||
}
|
||||
}, []);
|
||||
}, [setPersonas]);
|
||||
|
||||
useEffect(() => {
|
||||
loadPersonas();
|
||||
@@ -57,32 +65,35 @@ export function usePersonas() {
|
||||
};
|
||||
}, [refreshFromDisk]);
|
||||
|
||||
// biome-ignore lint/correctness/useExhaustiveDependencies: store is stable and should not trigger re-creation
|
||||
const createPersona = useCallback(async (req: CreatePersonaRequest) => {
|
||||
const persona = await api.createPersona(req);
|
||||
store.addPersona(persona);
|
||||
return persona;
|
||||
}, []);
|
||||
const createPersona = useCallback(
|
||||
async (req: CreatePersonaRequest) => {
|
||||
const persona = await api.createPersona(req);
|
||||
addPersona(persona);
|
||||
return persona;
|
||||
},
|
||||
[addPersona],
|
||||
);
|
||||
|
||||
// biome-ignore lint/correctness/useExhaustiveDependencies: store is stable and should not trigger re-creation
|
||||
const updatePersona = useCallback(
|
||||
async (id: string, req: UpdatePersonaRequest) => {
|
||||
const persona = await api.updatePersona(id, req);
|
||||
store.updatePersona(id, persona);
|
||||
updatePersonaInStore(id, persona);
|
||||
return persona;
|
||||
},
|
||||
[],
|
||||
[updatePersonaInStore],
|
||||
);
|
||||
|
||||
// biome-ignore lint/correctness/useExhaustiveDependencies: store is stable and should not trigger re-creation
|
||||
const deletePersona = useCallback(async (id: string) => {
|
||||
await api.deletePersona(id);
|
||||
store.removePersona(id);
|
||||
}, []);
|
||||
const deletePersona = useCallback(
|
||||
async (id: string) => {
|
||||
await api.deletePersona(id);
|
||||
removePersona(id);
|
||||
},
|
||||
[removePersona],
|
||||
);
|
||||
|
||||
return {
|
||||
personas: store.personas,
|
||||
isLoading: store.personasLoading,
|
||||
personas,
|
||||
isLoading: personasLoading,
|
||||
createPersona,
|
||||
updatePersona,
|
||||
deletePersona,
|
||||
|
||||
@@ -1,10 +1,11 @@
|
||||
import { useCallback } from "react";
|
||||
import { useAgentStore } from "../stores/agentStore";
|
||||
import { selectSelectedProvider } from "../stores/agentSelectors";
|
||||
|
||||
export function useProviderSelection() {
|
||||
const providers = useAgentStore((s) => s.providers);
|
||||
const providersLoading = useAgentStore((s) => s.providersLoading);
|
||||
const selectedProvider = useAgentStore((s) => s.selectedProvider);
|
||||
const selectedProvider = useAgentStore(selectSelectedProvider);
|
||||
const storeSetSelectedProvider = useAgentStore((s) => s.setSelectedProvider);
|
||||
|
||||
const setSelectedProvider = useCallback(
|
||||
|
||||
@@ -0,0 +1,9 @@
|
||||
import type { AgentStore } from "./agentStore";
|
||||
|
||||
export const selectPersonas = (state: AgentStore) => state.personas;
|
||||
|
||||
export const selectPersonasLoading = (state: AgentStore) =>
|
||||
state.personasLoading;
|
||||
|
||||
export const selectSelectedProvider = (state: AgentStore) =>
|
||||
state.selectedProvider;
|
||||
@@ -16,6 +16,10 @@ import {
|
||||
AlertDialogTitle,
|
||||
} from "@/shared/ui/alert-dialog";
|
||||
import { useAgentStore } from "@/features/agents/stores/agentStore";
|
||||
import {
|
||||
selectPersonas,
|
||||
selectPersonasLoading,
|
||||
} from "@/features/agents/stores/agentSelectors";
|
||||
import { PersonaGallery } from "@/features/agents/ui/PersonaGallery";
|
||||
import { PersonaEditor } from "@/features/agents/ui/PersonaEditor";
|
||||
import {
|
||||
@@ -41,8 +45,8 @@ export function AgentsView() {
|
||||
const [search, setSearch] = useState("");
|
||||
const [deletingPersona, setDeletingPersona] = useState<Persona | null>(null);
|
||||
|
||||
const personas = useAgentStore((s) => s.personas);
|
||||
const personasLoading = useAgentStore((s) => s.personasLoading);
|
||||
const personas = useAgentStore(selectPersonas);
|
||||
const personasLoading = useAgentStore(selectPersonasLoading);
|
||||
const personaEditorOpen = useAgentStore((s) => s.personaEditorOpen);
|
||||
const editingPersona = useAgentStore((s) => s.editingPersona);
|
||||
const personaEditorMode = useAgentStore((s) => s.personaEditorMode);
|
||||
|
||||
+2
-2
@@ -246,7 +246,7 @@ describe("useChatSessionController compaction behavior", () => {
|
||||
useChatStore
|
||||
.getState()
|
||||
.replaceTokenState("session-1", mockTokenState, true);
|
||||
useChatSessionStore.getState().updateSession("session-1", {
|
||||
useChatSessionStore.getState().patchSession("session-1", {
|
||||
providerId: "goose",
|
||||
});
|
||||
|
||||
@@ -300,7 +300,7 @@ describe("useChatSessionController compaction behavior", () => {
|
||||
useChatStore
|
||||
.getState()
|
||||
.replaceTokenState("session-1", mockTokenState, true);
|
||||
useChatSessionStore.getState().updateSession("session-1", {
|
||||
useChatSessionStore.getState().patchSession("session-1", {
|
||||
providerId: "goose",
|
||||
personaId: "persona-b",
|
||||
});
|
||||
|
||||
@@ -4,10 +4,12 @@ import { useChatSessionStore } from "../stores/chatSessionStore";
|
||||
import { clearReplayBuffer, getAndDeleteReplayBuffer } from "./replayBuffer";
|
||||
import {
|
||||
type ChatAttachmentDraft,
|
||||
type Message,
|
||||
createSystemNotificationMessage,
|
||||
createUserMessage,
|
||||
} from "@/shared/types/messages";
|
||||
import type { ChatState, TokenState } from "@/shared/types/chat";
|
||||
import { INITIAL_SESSION_CHAT_RUNTIME } from "@/shared/types/chat";
|
||||
import {
|
||||
acpSendMessage,
|
||||
acpCancelSession,
|
||||
@@ -18,7 +20,6 @@ import {
|
||||
getSessionTitleFromDraft,
|
||||
isDefaultChatTitle,
|
||||
} from "../lib/sessionTitle";
|
||||
import { findLastIndex } from "@/shared/lib/arrays";
|
||||
import { perfLog } from "@/shared/lib/perfLog";
|
||||
import {
|
||||
appendAttachmentPaths,
|
||||
@@ -28,10 +29,10 @@ import {
|
||||
import { sanitizeReplayMessages } from "../lib/replaySanitizer";
|
||||
import { i18n } from "@/shared/i18n";
|
||||
import type { ChatSendOptions } from "../types";
|
||||
import { buildSkillRetryOptions } from "../lib/skillSendPayload";
|
||||
|
||||
// TODO: Remove this fallback once goose2 has first-class /-commands.
|
||||
const MANUAL_COMPACT_TRIGGER = "/compact";
|
||||
const EMPTY_MESSAGES: Message[] = [];
|
||||
type CompactConversationResult = "completed" | "failed" | "skipped";
|
||||
|
||||
function createCompactionConfirmationMessage() {
|
||||
@@ -104,12 +105,28 @@ export function useChat(
|
||||
ensurePrepared?: (personaId?: string) => Promise<void>;
|
||||
},
|
||||
) {
|
||||
const store = useChatStore();
|
||||
const abortRef = useRef<AbortController | null>(null);
|
||||
|
||||
const messages = store.messagesBySession[sessionId] ?? [];
|
||||
const { chatState, tokenState, error, streamingMessageId } =
|
||||
store.getSessionRuntime(sessionId);
|
||||
const messages = useChatStore(
|
||||
(s) => s.messagesBySession[sessionId] ?? EMPTY_MESSAGES,
|
||||
);
|
||||
const runtime = useChatStore(
|
||||
(s) => s.sessionStateById[sessionId] ?? INITIAL_SESSION_CHAT_RUNTIME,
|
||||
);
|
||||
const setActiveSession = useChatStore((s) => s.setActiveSession);
|
||||
const addMessage = useChatStore((s) => s.addMessage);
|
||||
const setMessages = useChatStore((s) => s.setMessages);
|
||||
const clearMessages = useChatStore((s) => s.clearMessages);
|
||||
const setChatState = useChatStore((s) => s.setChatState);
|
||||
const setError = useChatStore((s) => s.setError);
|
||||
const setStreamingMessageId = useChatStore((s) => s.setStreamingMessageId);
|
||||
const setPendingAssistantProvider = useChatStore(
|
||||
(s) => s.setPendingAssistantProvider,
|
||||
);
|
||||
const clearDraft = useChatStore((s) => s.clearDraft);
|
||||
const setSessionLoading = useChatStore((s) => s.setSessionLoading);
|
||||
|
||||
const { chatState, tokenState, error, streamingMessageId } = runtime;
|
||||
const isStreaming = chatState === "streaming" || streamingMessageId !== null;
|
||||
|
||||
const resolvePersonaInfo = useCallback(
|
||||
@@ -166,8 +183,8 @@ export function useChat(
|
||||
systemPromptOverride ?? agent?.systemPrompt ?? undefined;
|
||||
|
||||
// Ensure active session
|
||||
store.setActiveSession(sessionId);
|
||||
store.setPendingAssistantProvider(sessionId, providerId);
|
||||
setActiveSession(sessionId);
|
||||
setPendingAssistantProvider(sessionId, providerId);
|
||||
|
||||
// Create and add user message
|
||||
const userMessage = createUserMessage(
|
||||
@@ -195,9 +212,9 @@ export function useChat(
|
||||
});
|
||||
}
|
||||
}
|
||||
store.addMessage(sessionId, userMessage);
|
||||
store.setChatState(sessionId, "thinking");
|
||||
store.setError(sessionId, null);
|
||||
addMessage(sessionId, userMessage);
|
||||
setChatState(sessionId, "thinking");
|
||||
setError(sessionId, null);
|
||||
|
||||
const sessionStore = useChatSessionStore.getState();
|
||||
const session = sessionStore.getSession(sessionId);
|
||||
@@ -208,19 +225,19 @@ export function useChat(
|
||||
// A better backend-generated title will overwrite this if it arrives
|
||||
// via the acp:session_info event.
|
||||
if (session && isDefaultChatTitle(session.title)) {
|
||||
sessionStore.updateSession(sessionId, {
|
||||
sessionStore.patchSession(sessionId, {
|
||||
title: getSessionTitleFromDraft(text, attachments),
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
} else {
|
||||
sessionStore.updateSession(sessionId, {
|
||||
sessionStore.patchSession(sessionId, {
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
}
|
||||
|
||||
options?.onMessageAccepted?.(sessionId);
|
||||
|
||||
store.clearDraft(sessionId);
|
||||
clearDraft(sessionId);
|
||||
|
||||
const abort = new AbortController();
|
||||
abortRef.current = abort;
|
||||
@@ -228,7 +245,7 @@ export function useChat(
|
||||
try {
|
||||
await options?.ensurePrepared?.(effectivePersonaInfo?.id);
|
||||
|
||||
store.setChatState(sessionId, "streaming");
|
||||
setChatState(sessionId, "streaming");
|
||||
const promptWithPaths = appendAttachmentPaths(text.trim(), attachments);
|
||||
const acpPrompt =
|
||||
promptWithPaths || (images?.length ? " " : promptWithPaths);
|
||||
@@ -251,11 +268,11 @@ export function useChat(
|
||||
`[perf:send] ${sid} acpSendMessage returned after ${(performance.now() - tAcp).toFixed(1)}ms (total sendMessage ${(performance.now() - tSendStart).toFixed(1)}ms)`,
|
||||
);
|
||||
|
||||
store.setChatState(sessionId, "idle");
|
||||
store.setStreamingMessageId(sessionId, null);
|
||||
setChatState(sessionId, "idle");
|
||||
setStreamingMessageId(sessionId, null);
|
||||
} catch (err) {
|
||||
if (err instanceof DOMException && err.name === "AbortError") {
|
||||
store.setChatState(sessionId, "idle");
|
||||
setChatState(sessionId, "idle");
|
||||
} else {
|
||||
const errorMessage = getErrorMessage(err);
|
||||
const liveStore = useChatStore.getState();
|
||||
@@ -278,18 +295,24 @@ export function useChat(
|
||||
sessionId,
|
||||
createSystemNotificationMessage(errorMessage, "error"),
|
||||
);
|
||||
store.setError(sessionId, errorMessage);
|
||||
store.setChatState(sessionId, "idle");
|
||||
store.setStreamingMessageId(sessionId, null);
|
||||
setError(sessionId, errorMessage);
|
||||
setChatState(sessionId, "idle");
|
||||
setStreamingMessageId(sessionId, null);
|
||||
}
|
||||
store.setPendingAssistantProvider(sessionId, null);
|
||||
setPendingAssistantProvider(sessionId, null);
|
||||
} finally {
|
||||
abortRef.current = null;
|
||||
}
|
||||
},
|
||||
[
|
||||
sessionId,
|
||||
store,
|
||||
setActiveSession,
|
||||
setPendingAssistantProvider,
|
||||
addMessage,
|
||||
setChatState,
|
||||
setError,
|
||||
clearDraft,
|
||||
setStreamingMessageId,
|
||||
providerOverride,
|
||||
systemPromptOverride,
|
||||
resolvePersonaInfo,
|
||||
@@ -303,9 +326,9 @@ export function useChat(
|
||||
.getState()
|
||||
.getSessionRuntime(sessionId).streamingMessageId;
|
||||
|
||||
store.setChatState(sessionId, "idle");
|
||||
store.setStreamingMessageId(sessionId, null);
|
||||
store.setPendingAssistantProvider(sessionId, null);
|
||||
setChatState(sessionId, "idle");
|
||||
setStreamingMessageId(sessionId, null);
|
||||
setPendingAssistantProvider(sessionId, null);
|
||||
// Cancel the backend ACP session to stop orphaned streaming events
|
||||
acpCancelSession(sessionId)
|
||||
.then((wasCancelled) => {
|
||||
@@ -316,50 +339,26 @@ export function useChat(
|
||||
.catch(() => {
|
||||
// Best-effort cancellation — ignore errors
|
||||
});
|
||||
}, [store, sessionId]);
|
||||
|
||||
const retryLastMessage = useCallback(async () => {
|
||||
const sessionMessages = store.messagesBySession[sessionId] ?? [];
|
||||
// Find the last user message
|
||||
const lastUserIndex = findLastIndex(
|
||||
sessionMessages,
|
||||
(m) => m.role === "user",
|
||||
);
|
||||
if (lastUserIndex === -1) return;
|
||||
|
||||
const lastUserMessage = sessionMessages[lastUserIndex];
|
||||
|
||||
// Remove all messages after (and including) the last assistant response
|
||||
const messagesToKeep = sessionMessages.slice(0, lastUserIndex);
|
||||
store.setMessages(sessionId, messagesToKeep);
|
||||
|
||||
// Extract the text and resend
|
||||
const textContent = lastUserMessage.content.find((c) => c.type === "text");
|
||||
if (textContent && "text" in textContent) {
|
||||
const targetPersonaId = lastUserMessage.metadata?.targetPersonaId;
|
||||
const targetPersonaName = lastUserMessage.metadata?.targetPersonaName;
|
||||
const retryOptions = buildSkillRetryOptions(
|
||||
textContent.text,
|
||||
lastUserMessage.metadata?.chips,
|
||||
);
|
||||
await sendMessage(
|
||||
textContent.text || (retryOptions ? " " : ""),
|
||||
targetPersonaId
|
||||
? { id: targetPersonaId, name: targetPersonaName }
|
||||
: undefined,
|
||||
undefined,
|
||||
retryOptions,
|
||||
);
|
||||
}
|
||||
}, [sessionId, store, sendMessage]);
|
||||
}, [
|
||||
setChatState,
|
||||
setPendingAssistantProvider,
|
||||
setStreamingMessageId,
|
||||
sessionId,
|
||||
]);
|
||||
|
||||
const clearChat = useCallback(() => {
|
||||
abortRef.current?.abort();
|
||||
store.clearMessages(sessionId);
|
||||
store.setChatState(sessionId, "idle");
|
||||
store.setStreamingMessageId(sessionId, null);
|
||||
store.setPendingAssistantProvider(sessionId, null);
|
||||
}, [sessionId, store]);
|
||||
clearMessages(sessionId);
|
||||
setChatState(sessionId, "idle");
|
||||
setStreamingMessageId(sessionId, null);
|
||||
setPendingAssistantProvider(sessionId, null);
|
||||
}, [
|
||||
sessionId,
|
||||
clearMessages,
|
||||
setChatState,
|
||||
setStreamingMessageId,
|
||||
setPendingAssistantProvider,
|
||||
]);
|
||||
|
||||
const getWorkingDir = useCallback(
|
||||
() =>
|
||||
@@ -381,25 +380,25 @@ export function useChat(
|
||||
overridePersona?.name,
|
||||
);
|
||||
|
||||
store.setActiveSession(sessionId);
|
||||
store.setChatState(sessionId, "compacting");
|
||||
store.setStreamingMessageId(sessionId, null);
|
||||
store.setError(sessionId, null);
|
||||
setActiveSession(sessionId);
|
||||
setChatState(sessionId, "compacting");
|
||||
setStreamingMessageId(sessionId, null);
|
||||
setError(sessionId, null);
|
||||
|
||||
try {
|
||||
await options?.ensurePrepared?.(effectivePersonaInfo?.id);
|
||||
} catch (err) {
|
||||
const errorMessage = getErrorMessage(err);
|
||||
store.addMessage(
|
||||
addMessage(
|
||||
sessionId,
|
||||
createSystemNotificationMessage(errorMessage, "error"),
|
||||
);
|
||||
store.setError(sessionId, errorMessage);
|
||||
store.setChatState(sessionId, "idle");
|
||||
setError(sessionId, errorMessage);
|
||||
setChatState(sessionId, "idle");
|
||||
return "failed" as CompactConversationResult;
|
||||
}
|
||||
|
||||
store.setSessionLoading(sessionId, true);
|
||||
setSessionLoading(sessionId, true);
|
||||
clearReplayBuffer(sessionId);
|
||||
|
||||
try {
|
||||
@@ -415,37 +414,50 @@ export function useChat(
|
||||
const workingDir = getWorkingDir();
|
||||
await acpLoadSession(sessionId, workingDir);
|
||||
|
||||
store.setSessionLoading(sessionId, false);
|
||||
setSessionLoading(sessionId, false);
|
||||
|
||||
const buffer = getAndDeleteReplayBuffer(sessionId);
|
||||
if (buffer) {
|
||||
store.setMessages(sessionId, [
|
||||
setMessages(sessionId, [
|
||||
...sanitizeReplayMessages(buffer),
|
||||
createCompactionConfirmationMessage(),
|
||||
]);
|
||||
} else {
|
||||
store.addMessage(sessionId, createCompactionConfirmationMessage());
|
||||
addMessage(sessionId, createCompactionConfirmationMessage());
|
||||
}
|
||||
return "completed" as CompactConversationResult;
|
||||
} catch (err) {
|
||||
clearReplayBuffer(sessionId);
|
||||
store.setSessionLoading(sessionId, false);
|
||||
setSessionLoading(sessionId, false);
|
||||
|
||||
const errorMessage = getErrorMessage(err);
|
||||
store.addMessage(
|
||||
addMessage(
|
||||
sessionId,
|
||||
createSystemNotificationMessage(errorMessage, "error"),
|
||||
);
|
||||
store.setError(sessionId, errorMessage);
|
||||
setError(sessionId, errorMessage);
|
||||
return "failed" as CompactConversationResult;
|
||||
} finally {
|
||||
store.setChatState(sessionId, "idle");
|
||||
store.setStreamingMessageId(sessionId, null);
|
||||
store.setPendingAssistantProvider(sessionId, null);
|
||||
store.setSessionLoading(sessionId, false);
|
||||
setChatState(sessionId, "idle");
|
||||
setStreamingMessageId(sessionId, null);
|
||||
setPendingAssistantProvider(sessionId, null);
|
||||
setSessionLoading(sessionId, false);
|
||||
}
|
||||
},
|
||||
[getWorkingDir, options, resolvePersonaInfo, sessionId, store],
|
||||
[
|
||||
getWorkingDir,
|
||||
options,
|
||||
resolvePersonaInfo,
|
||||
sessionId,
|
||||
setActiveSession,
|
||||
setChatState,
|
||||
setStreamingMessageId,
|
||||
setError,
|
||||
addMessage,
|
||||
setSessionLoading,
|
||||
setMessages,
|
||||
setPendingAssistantProvider,
|
||||
],
|
||||
);
|
||||
|
||||
const stopStreaming = stopGeneration;
|
||||
@@ -459,7 +471,6 @@ export function useChat(
|
||||
sendMessage,
|
||||
stopGeneration,
|
||||
stopStreaming,
|
||||
retryLastMessage,
|
||||
clearChat,
|
||||
compactConversation,
|
||||
isStreaming,
|
||||
|
||||
@@ -8,8 +8,10 @@ import { useMessageQueue } from "./useMessageQueue";
|
||||
import { useChatStore } from "../stores/chatStore";
|
||||
import { useChatSessionStore } from "../stores/chatSessionStore";
|
||||
import { useAgentStore } from "@/features/agents/stores/agentStore";
|
||||
import { selectPersonas } from "@/features/agents/stores/agentSelectors";
|
||||
import { useProviderSelection } from "@/features/agents/hooks/useProviderSelection";
|
||||
import { useProjectStore } from "@/features/projects/stores/projectStore";
|
||||
import { selectProjects } from "@/features/projects/stores/projectSelectors";
|
||||
import { resolveAgentProviderCatalogIdStrictFromEntries } from "@/features/providers/providerCatalog";
|
||||
import { useProviderCatalogStore } from "@/features/providers/stores/providerCatalogStore";
|
||||
import {
|
||||
@@ -25,6 +27,7 @@ import {
|
||||
} from "../lib/autoCompact";
|
||||
import { resolveSessionCwd } from "@/features/projects/lib/sessionCwdSelection";
|
||||
import { acpPrepareSession, acpSetModel } from "@/shared/api/acp";
|
||||
import { updateSessionProject } from "../stores/chatSessionOperations";
|
||||
import {
|
||||
useResolvedAgentModelPicker,
|
||||
type PreferredModelSelection,
|
||||
@@ -51,7 +54,7 @@ export function useChatSessionController({
|
||||
selectedProvider: globalSelectedProvider,
|
||||
setSelectedProvider: setGlobalSelectedProvider,
|
||||
} = useProviderSelection();
|
||||
const personas = useAgentStore((s) => s.personas);
|
||||
const personas = useAgentStore(selectPersonas);
|
||||
const session = useChatSessionStore((s) =>
|
||||
sessionId
|
||||
? s.sessions.find((candidate) => candidate.id === sessionId)
|
||||
@@ -63,7 +66,7 @@ export function useChatSessionController({
|
||||
const clearActiveWorkspace = useChatSessionStore(
|
||||
(s) => s.clearActiveWorkspace,
|
||||
);
|
||||
const projects = useProjectStore((s) => s.projects);
|
||||
const projects = useProjectStore(selectProjects);
|
||||
const projectsLoading = useProjectStore((s) => s.loading);
|
||||
const catalogEntries = useProviderCatalogStore((s) => s.entries);
|
||||
const [pendingPersonaId, setPendingPersonaId] = useState<string | null>();
|
||||
@@ -184,7 +187,7 @@ export function useChatSessionController({
|
||||
}
|
||||
|
||||
await acpSetModel(sessionId, modelSelection.id);
|
||||
sessionStore.updateSession(sessionId, {
|
||||
sessionStore.patchSession(sessionId, {
|
||||
modelId: modelSelection.id,
|
||||
modelName: modelSelection.name,
|
||||
});
|
||||
@@ -315,16 +318,18 @@ export function useChatSessionController({
|
||||
.projects.find((candidate) => candidate.id === projectId) ??
|
||||
null);
|
||||
|
||||
useChatSessionStore.getState().updateSession(sessionId, { projectId });
|
||||
if (!selectedProvider) {
|
||||
return;
|
||||
}
|
||||
void prepareCurrentSession(
|
||||
selectedProvider,
|
||||
nextProject,
|
||||
activeWorkspace?.path,
|
||||
effectiveModelSelection,
|
||||
).catch((error) => {
|
||||
void (async () => {
|
||||
await updateSessionProject(sessionId, projectId);
|
||||
if (!selectedProvider) {
|
||||
return;
|
||||
}
|
||||
await prepareCurrentSession(
|
||||
selectedProvider,
|
||||
nextProject,
|
||||
activeWorkspace?.path,
|
||||
effectiveModelSelection,
|
||||
);
|
||||
})().catch((error) => {
|
||||
console.error("Failed to update ACP session working directory:", error);
|
||||
});
|
||||
},
|
||||
@@ -374,7 +379,7 @@ export function useChatSessionController({
|
||||
}
|
||||
useChatSessionStore
|
||||
.getState()
|
||||
.updateSession(sessionId, { personaId: personaId ?? undefined });
|
||||
.patchSession(sessionId, { personaId: personaId ?? undefined });
|
||||
},
|
||||
[
|
||||
handleProviderChange,
|
||||
@@ -395,7 +400,7 @@ export function useChatSessionController({
|
||||
if (sessionId) {
|
||||
useChatSessionStore
|
||||
.getState()
|
||||
.updateSession(sessionId, { personaId: undefined });
|
||||
.patchSession(sessionId, { personaId: undefined });
|
||||
} else {
|
||||
setPendingPersonaId(undefined);
|
||||
}
|
||||
@@ -703,7 +708,6 @@ export function useChatSessionController({
|
||||
const patch: {
|
||||
providerId?: string;
|
||||
personaId?: string | undefined;
|
||||
projectId?: string | null;
|
||||
modelId?: string | undefined;
|
||||
modelName?: string | undefined;
|
||||
} = {};
|
||||
@@ -716,13 +720,14 @@ export function useChatSessionController({
|
||||
if (hasPendingPersona) {
|
||||
patch.personaId = nextPersonaId;
|
||||
}
|
||||
if (hasPendingProject) {
|
||||
patch.projectId = nextProjectId ?? null;
|
||||
if (Object.keys(patch).length > 0) {
|
||||
useChatSessionStore.getState().patchSession(sessionId, patch);
|
||||
}
|
||||
|
||||
useChatSessionStore.getState().updateSession(sessionId, patch);
|
||||
|
||||
try {
|
||||
if (hasPendingProject) {
|
||||
await updateSessionProject(sessionId, nextProjectId ?? null);
|
||||
}
|
||||
await prepareCurrentSession(
|
||||
nextProviderId,
|
||||
nextProject,
|
||||
|
||||
@@ -310,7 +310,7 @@ export function useResolvedAgentModelPicker({
|
||||
setGlobalSelectedProvider(nextProviderId);
|
||||
}
|
||||
|
||||
useChatSessionStore.getState().updateSession(sessionId, {
|
||||
useChatSessionStore.getState().patchSession(sessionId, {
|
||||
modelId,
|
||||
modelName,
|
||||
});
|
||||
@@ -335,7 +335,7 @@ export function useResolvedAgentModelPicker({
|
||||
} else {
|
||||
clearStoredModelPreference(selectedAgentId);
|
||||
}
|
||||
useChatSessionStore.getState().updateSession(sessionId, {
|
||||
useChatSessionStore.getState().patchSession(sessionId, {
|
||||
providerId: previousProviderId,
|
||||
modelId: previousModelId,
|
||||
modelName: previousModelName,
|
||||
|
||||
@@ -2,7 +2,6 @@ import {
|
||||
formatSkillInstructionPrompt,
|
||||
type SkillCommandMatch,
|
||||
} from "@/features/skills/lib/skillChatPrompt";
|
||||
import type { MessageChip } from "@/shared/types/messages";
|
||||
import type { ChatSendOptions, ChatSkillDraft } from "../types";
|
||||
|
||||
interface SkillSendPayload {
|
||||
@@ -50,19 +49,3 @@ export function buildSkillSendPayload(
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
export function buildSkillRetryOptions(
|
||||
text: string,
|
||||
chips?: MessageChip[],
|
||||
): ChatSendOptions | undefined {
|
||||
const skillChips = chips?.filter((chip) => chip.type === "skill") ?? [];
|
||||
if (skillChips.length === 0) return undefined;
|
||||
|
||||
return {
|
||||
displayText: text,
|
||||
assistantPrompt: formatSkillInstructionPrompt(
|
||||
skillChips.map((chip) => ({ name: chip.label })),
|
||||
),
|
||||
chips: skillChips,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { useChatSessionStore, type ChatSession } from "../chatSessionStore";
|
||||
import {
|
||||
updateSessionProject,
|
||||
updateSessionTitle,
|
||||
} from "../chatSessionOperations";
|
||||
|
||||
const mockRenameSession = vi.fn();
|
||||
const mockUpdateSessionProject = vi.fn();
|
||||
|
||||
vi.mock("@/shared/api/acpApi", () => ({
|
||||
renameSession: (...args: unknown[]) => mockRenameSession(...args),
|
||||
updateSessionProject: (...args: unknown[]) =>
|
||||
mockUpdateSessionProject(...args),
|
||||
}));
|
||||
|
||||
function resetStore() {
|
||||
useChatSessionStore.setState({
|
||||
sessions: [],
|
||||
activeSessionId: null,
|
||||
isLoading: false,
|
||||
hasHydratedSessions: false,
|
||||
contextPanelOpenBySession: {},
|
||||
activeWorkspaceBySession: {},
|
||||
});
|
||||
}
|
||||
|
||||
function seedSession(overrides: Partial<ChatSession> = {}) {
|
||||
useChatSessionStore.setState({
|
||||
sessions: [
|
||||
{
|
||||
id: "session-1",
|
||||
title: "Original Title",
|
||||
createdAt: "2026-04-01T00:00:00.000Z",
|
||||
updatedAt: "2026-04-01T00:00:00.000Z",
|
||||
messageCount: 0,
|
||||
...overrides,
|
||||
},
|
||||
],
|
||||
});
|
||||
}
|
||||
|
||||
describe("chatSessionOperations", () => {
|
||||
beforeEach(() => {
|
||||
resetStore();
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
describe("updateSessionTitle", () => {
|
||||
it("renames in backend before patching local state", async () => {
|
||||
seedSession({ userSetName: false });
|
||||
mockRenameSession.mockResolvedValue(undefined);
|
||||
|
||||
await updateSessionTitle("session-1", "Manual Title");
|
||||
|
||||
expect(mockRenameSession).toHaveBeenCalledWith(
|
||||
"session-1",
|
||||
"Manual Title",
|
||||
);
|
||||
expect(
|
||||
useChatSessionStore.getState().getSession("session-1"),
|
||||
).toMatchObject({
|
||||
title: "Manual Title",
|
||||
userSetName: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("does not patch local state when backend rename fails", async () => {
|
||||
seedSession({ userSetName: false });
|
||||
mockRenameSession.mockRejectedValue(new Error("rename failed"));
|
||||
|
||||
await expect(
|
||||
updateSessionTitle("session-1", "Manual Title"),
|
||||
).rejects.toThrow("rename failed");
|
||||
|
||||
expect(
|
||||
useChatSessionStore.getState().getSession("session-1"),
|
||||
).toMatchObject({
|
||||
title: "Original Title",
|
||||
userSetName: false,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("updateSessionProject", () => {
|
||||
it("updates project in backend before patching local state", async () => {
|
||||
seedSession({ projectId: "project-old" });
|
||||
mockUpdateSessionProject.mockResolvedValue(undefined);
|
||||
|
||||
await updateSessionProject("session-1", "project-new");
|
||||
|
||||
expect(mockUpdateSessionProject).toHaveBeenCalledWith(
|
||||
"session-1",
|
||||
"project-new",
|
||||
);
|
||||
expect(
|
||||
useChatSessionStore.getState().getSession("session-1")?.projectId,
|
||||
).toBe("project-new");
|
||||
});
|
||||
|
||||
it("does not patch local state when backend project update fails", async () => {
|
||||
seedSession({ projectId: "project-old" });
|
||||
mockUpdateSessionProject.mockRejectedValue(new Error("project failed"));
|
||||
|
||||
await expect(
|
||||
updateSessionProject("session-1", "project-new"),
|
||||
).rejects.toThrow("project failed");
|
||||
|
||||
expect(
|
||||
useChatSessionStore.getState().getSession("session-1")?.projectId,
|
||||
).toBe("project-old");
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -225,54 +225,32 @@ describe("chatSessionStore", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("updateSession", () => {
|
||||
it("updates session properties", () => {
|
||||
describe("patchSession", () => {
|
||||
it("patches session properties while preserving updatedAt when omitted", () => {
|
||||
const session = seedSession();
|
||||
const originalUpdatedAt = session.updatedAt;
|
||||
|
||||
useChatSessionStore.getState().updateSession(session.id, {
|
||||
useChatSessionStore.getState().patchSession(session.id, {
|
||||
title: "Updated Title",
|
||||
projectId: "new-project",
|
||||
});
|
||||
|
||||
const updated = useChatSessionStore.getState().getSession(session.id);
|
||||
expect(updated?.title).toBe("Updated Title");
|
||||
expect(updated?.projectId).toBe("new-project");
|
||||
});
|
||||
|
||||
it("preserves updatedAt when not explicitly provided in patch", () => {
|
||||
const session = seedSession();
|
||||
const originalUpdatedAt = session.updatedAt;
|
||||
|
||||
vi.useFakeTimers();
|
||||
vi.advanceTimersByTime(1000);
|
||||
|
||||
useChatSessionStore.getState().updateSession(session.id, {
|
||||
title: "New Title",
|
||||
expect(updated).toMatchObject({
|
||||
title: "Updated Title",
|
||||
projectId: "new-project",
|
||||
updatedAt: originalUpdatedAt,
|
||||
});
|
||||
|
||||
vi.useRealTimers();
|
||||
|
||||
const updated = useChatSessionStore.getState().getSession(session.id);
|
||||
expect(updated?.updatedAt).toBe(originalUpdatedAt);
|
||||
});
|
||||
|
||||
it("updates updatedAt when explicitly provided in patch", () => {
|
||||
const session = seedSession();
|
||||
const originalUpdatedAt = session.updatedAt;
|
||||
|
||||
vi.useFakeTimers();
|
||||
vi.advanceTimersByTime(1000);
|
||||
|
||||
const newTimestamp = new Date().toISOString();
|
||||
useChatSessionStore.getState().updateSession(session.id, {
|
||||
title: "New Title",
|
||||
const newTimestamp = "2026-04-01T00:01:00.000Z";
|
||||
useChatSessionStore.getState().patchSession(session.id, {
|
||||
updatedAt: newTimestamp,
|
||||
});
|
||||
|
||||
vi.useRealTimers();
|
||||
|
||||
const updated = useChatSessionStore.getState().getSession(session.id);
|
||||
expect(updated?.updatedAt).not.toBe(originalUpdatedAt);
|
||||
expect(updated?.updatedAt).toBe(newTimestamp);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,7 @@
|
||||
import type { ChatStore } from "./chatStore";
|
||||
|
||||
export const selectMessagesBySession = (state: ChatStore) =>
|
||||
state.messagesBySession;
|
||||
|
||||
export const selectSessionStateById = (state: ChatStore) =>
|
||||
state.sessionStateById;
|
||||
@@ -0,0 +1,28 @@
|
||||
import {
|
||||
renameSession,
|
||||
updateSessionProject as updateSessionProjectApi,
|
||||
} from "@/shared/api/acpApi";
|
||||
import { useChatSessionStore } from "./chatSessionStore";
|
||||
|
||||
export async function updateSessionTitle(
|
||||
sessionId: string,
|
||||
title: string,
|
||||
): Promise<void> {
|
||||
await renameSession(sessionId, title);
|
||||
|
||||
useChatSessionStore.getState().patchSession(sessionId, {
|
||||
title,
|
||||
userSetName: true,
|
||||
});
|
||||
}
|
||||
|
||||
export async function updateSessionProject(
|
||||
sessionId: string,
|
||||
projectId: string | null,
|
||||
): Promise<void> {
|
||||
await updateSessionProjectApi(sessionId, projectId);
|
||||
|
||||
useChatSessionStore.getState().patchSession(sessionId, {
|
||||
projectId,
|
||||
});
|
||||
}
|
||||
@@ -0,0 +1,12 @@
|
||||
import type { ChatSessionStore } from "./chatSessionStore";
|
||||
|
||||
export const selectSessions = (state: ChatSessionStore) => state.sessions;
|
||||
|
||||
export const selectActiveSessionId = (state: ChatSessionStore) =>
|
||||
state.activeSessionId;
|
||||
|
||||
export const selectHasHydratedSessions = (state: ChatSessionStore) =>
|
||||
state.hasHydratedSessions;
|
||||
|
||||
export const selectSessionsLoading = (state: ChatSessionStore) =>
|
||||
state.isLoading;
|
||||
@@ -12,8 +12,6 @@ import {
|
||||
import {
|
||||
archiveSession as acpArchiveSession,
|
||||
unarchiveSession as acpUnarchiveSession,
|
||||
renameSession as acpRenameSession,
|
||||
updateSessionProject,
|
||||
} from "@/shared/api/acpApi";
|
||||
|
||||
export interface ChatSession {
|
||||
@@ -77,7 +75,7 @@ interface CreateSessionOpts {
|
||||
interface ChatSessionStoreActions {
|
||||
createSession: (opts?: CreateSessionOpts) => Promise<ChatSession>;
|
||||
loadSessions: () => Promise<void>;
|
||||
updateSession: (id: string, patch: Partial<ChatSession>) => void;
|
||||
patchSession: (id: string, patch: Partial<ChatSession>) => void;
|
||||
addSession: (session: ChatSession) => void;
|
||||
archiveSession: (id: string) => Promise<void>;
|
||||
unarchiveSession: (id: string) => Promise<void>;
|
||||
@@ -195,7 +193,7 @@ export const useChatSessionStore = create<ChatSessionStore>((set, get) => ({
|
||||
}
|
||||
},
|
||||
|
||||
updateSession: (id, patch) => {
|
||||
patchSession: (id, patch) => {
|
||||
set((state) => ({
|
||||
sessions: state.sessions.map((session) =>
|
||||
session.id === id
|
||||
@@ -207,29 +205,6 @@ export const useChatSessionStore = create<ChatSessionStore>((set, get) => ({
|
||||
: session,
|
||||
),
|
||||
}));
|
||||
|
||||
const updatedSession = get().sessions.find((session) => session.id === id);
|
||||
|
||||
// Persist title rename to backend
|
||||
if (
|
||||
"title" in patch &&
|
||||
"userSetName" in patch &&
|
||||
patch.userSetName &&
|
||||
updatedSession &&
|
||||
patch.title
|
||||
) {
|
||||
acpRenameSession(updatedSession.id, patch.title).catch((err: unknown) =>
|
||||
console.error("Failed to rename session in backend:", err),
|
||||
);
|
||||
}
|
||||
|
||||
// Persist projectId change to backend
|
||||
if ("projectId" in patch && updatedSession) {
|
||||
updateSessionProject(updatedSession.id, patch.projectId ?? null).catch(
|
||||
(err: unknown) =>
|
||||
console.error("Failed to update session project in backend:", err),
|
||||
);
|
||||
}
|
||||
},
|
||||
|
||||
addSession: (session) => {
|
||||
|
||||
@@ -0,0 +1,3 @@
|
||||
import type { ProjectStore } from "./projectStore";
|
||||
|
||||
export const selectProjects = (state: ProjectStore) => state.projects;
|
||||
@@ -34,7 +34,7 @@ function persistProjects(projects: ProjectInfo[]): void {
|
||||
}
|
||||
}
|
||||
|
||||
interface ProjectState {
|
||||
export interface ProjectStore {
|
||||
projects: ProjectInfo[];
|
||||
loading: boolean;
|
||||
activeProjectId: string | null;
|
||||
@@ -70,7 +70,7 @@ interface ProjectState {
|
||||
getActiveProject: () => ProjectInfo | null;
|
||||
}
|
||||
|
||||
export const useProjectStore = create<ProjectState>((set, get) => ({
|
||||
export const useProjectStore = create<ProjectStore>((set, get) => ({
|
||||
projects: loadCachedProjects(),
|
||||
loading: false,
|
||||
activeProjectId: null,
|
||||
|
||||
@@ -12,8 +12,11 @@ import {
|
||||
getVisibleSessions,
|
||||
useChatSessionStore,
|
||||
} from "@/features/chat/stores/chatSessionStore";
|
||||
import { selectSessions } from "@/features/chat/stores/chatSessionSelectors";
|
||||
import { useChatStore } from "@/features/chat/stores/chatStore";
|
||||
import { selectMessagesBySession } from "@/features/chat/stores/chatSelectors";
|
||||
import { useProjectStore } from "@/features/projects/stores/projectStore";
|
||||
import { selectProjects } from "@/features/projects/stores/projectSelectors";
|
||||
import {
|
||||
acpDuplicateSession,
|
||||
acpExportSession,
|
||||
@@ -41,8 +44,8 @@ export function SessionHistoryView({
|
||||
onArchiveChat,
|
||||
}: SessionHistoryViewProps) {
|
||||
const { t, i18n } = useTranslation(["sessions", "common"]);
|
||||
const sessions = useChatSessionStore((s) => s.sessions);
|
||||
const messagesBySession = useChatStore((s) => s.messagesBySession);
|
||||
const sessions = useChatSessionStore(selectSessions);
|
||||
const messagesBySession = useChatStore(selectMessagesBySession);
|
||||
const loadSessions = useChatSessionStore((s) => s.loadSessions);
|
||||
const activeSessions = useMemo(
|
||||
() =>
|
||||
@@ -59,7 +62,7 @@ export function SessionHistoryView({
|
||||
[],
|
||||
);
|
||||
|
||||
const projects = useProjectStore((s) => s.projects);
|
||||
const projects = useProjectStore(selectProjects);
|
||||
const getProjectName = useCallback(
|
||||
(projectId: string) => projects.find((p) => p.id === projectId)?.name,
|
||||
[projects],
|
||||
|
||||
@@ -17,13 +17,20 @@ import { cn } from "@/shared/lib/cn";
|
||||
import type { AppView } from "@/app/AppShell";
|
||||
import type { ProjectInfo } from "@/features/projects/api/projects";
|
||||
import { useChatStore } from "@/features/chat/stores/chatStore";
|
||||
import {
|
||||
selectMessagesBySession,
|
||||
selectSessionStateById,
|
||||
} from "@/features/chat/stores/chatSelectors";
|
||||
import { INITIAL_SESSION_CHAT_RUNTIME } from "@/shared/types/chat";
|
||||
import {
|
||||
getVisibleSessions,
|
||||
useChatSessionStore,
|
||||
} from "@/features/chat/stores/chatSessionStore";
|
||||
import { selectSessions } from "@/features/chat/stores/chatSessionSelectors";
|
||||
import { isSessionRunning } from "@/features/chat/lib/sessionActivity";
|
||||
import { useAgentStore } from "@/features/agents/stores/agentStore";
|
||||
import { useProjectStore } from "@/features/projects/stores/projectStore";
|
||||
import { selectProjects } from "@/features/projects/stores/projectSelectors";
|
||||
import { Button } from "@/shared/ui/button";
|
||||
import { useSessionSearch } from "@/features/sessions/hooks/useSessionSearch";
|
||||
import { SidebarProjectsSection } from "./SidebarProjectsSection";
|
||||
@@ -101,12 +108,12 @@ export function Sidebar({
|
||||
}
|
||||
});
|
||||
|
||||
const chatStore = useChatStore();
|
||||
const { sessions } = useChatSessionStore();
|
||||
const visibleSessions = getVisibleSessions(
|
||||
sessions,
|
||||
chatStore.messagesBySession,
|
||||
);
|
||||
const messagesBySession = useChatStore(selectMessagesBySession);
|
||||
const sessionStateById = useChatStore(selectSessionStateById);
|
||||
const sessions = useChatSessionStore(selectSessions);
|
||||
const getPersonaById = useAgentStore((s) => s.getPersonaById);
|
||||
const projectStoreProjects = useProjectStore(selectProjects);
|
||||
const visibleSessions = getVisibleSessions(sessions, messagesBySession);
|
||||
const activeSessions = visibleSessions.filter(
|
||||
(session) => !session.archivedAt,
|
||||
);
|
||||
@@ -162,7 +169,8 @@ export function Sidebar({
|
||||
const standalone: SessionItem[] = [];
|
||||
for (const session of visibleSessions) {
|
||||
if (session.archivedAt) continue;
|
||||
const runtime = chatStore.getSessionRuntime(session.id);
|
||||
const runtime =
|
||||
sessionStateById[session.id] ?? INITIAL_SESSION_CHAT_RUNTIME;
|
||||
const item: SessionItem = {
|
||||
id: session.id,
|
||||
title: session.title,
|
||||
@@ -194,15 +202,11 @@ export function Sidebar({
|
||||
return { byProject, standalone: limitedStandalone };
|
||||
})();
|
||||
|
||||
const agentStoreState = useAgentStore();
|
||||
const projectStoreState = useProjectStore();
|
||||
|
||||
const sidebarResolvers = {
|
||||
getPersonaName: (personaId: string) =>
|
||||
agentStoreState.getPersonaById(personaId)?.displayName,
|
||||
getPersonaById(personaId)?.displayName,
|
||||
getProjectName: (projectId: string) =>
|
||||
projectStoreState.projects.find((p: { id: string }) => p.id === projectId)
|
||||
?.name,
|
||||
projectStoreProjects.find((p) => p.id === projectId)?.name,
|
||||
};
|
||||
const sidebarSearch = useSessionSearch({
|
||||
sessions: activeSessions,
|
||||
|
||||
@@ -13,33 +13,34 @@ const mockSessions: Array<{
|
||||
}> = [];
|
||||
|
||||
vi.mock("@/features/chat/stores/chatStore", () => ({
|
||||
useChatStore: () => ({
|
||||
messagesBySession: {},
|
||||
getSessionRuntime: () => ({
|
||||
chatState: "idle",
|
||||
hasUnread: false,
|
||||
useChatStore: (selector: (state: unknown) => unknown) =>
|
||||
selector({
|
||||
messagesBySession: {},
|
||||
sessionStateById: {},
|
||||
}),
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock("@/features/chat/stores/chatSessionStore", () => ({
|
||||
getVisibleSessions: (sessions: typeof mockSessions) =>
|
||||
sessions.filter((session) => session.messageCount > 0),
|
||||
useChatSessionStore: () => ({
|
||||
sessions: mockSessions,
|
||||
}),
|
||||
useChatSessionStore: (selector: (state: unknown) => unknown) =>
|
||||
selector({
|
||||
sessions: mockSessions,
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock("@/features/agents/stores/agentStore", () => ({
|
||||
useAgentStore: () => ({
|
||||
getPersonaById: () => undefined,
|
||||
}),
|
||||
useAgentStore: (selector: (state: unknown) => unknown) =>
|
||||
selector({
|
||||
getPersonaById: () => undefined,
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock("@/features/projects/stores/projectStore", () => ({
|
||||
useProjectStore: () => ({
|
||||
projects: [],
|
||||
}),
|
||||
useProjectStore: (selector: (state: unknown) => unknown) =>
|
||||
selector({
|
||||
projects: [],
|
||||
}),
|
||||
}));
|
||||
|
||||
describe("Sidebar", () => {
|
||||
|
||||
@@ -3,6 +3,7 @@ import { useTranslation } from "react-i18next";
|
||||
import { Plus, Upload } from "lucide-react";
|
||||
import { toast } from "sonner";
|
||||
import { useProjectStore } from "@/features/projects/stores/projectStore";
|
||||
import { selectProjects } from "@/features/projects/stores/projectSelectors";
|
||||
import { Button } from "@/shared/ui/button";
|
||||
import { PageHeader, PageShell } from "@/shared/ui/page-shell";
|
||||
import { revealInFileManager } from "@/shared/lib/fileManager";
|
||||
@@ -32,7 +33,7 @@ interface SkillsViewProps {
|
||||
|
||||
export function SkillsView({ onStartChatWithSkill }: SkillsViewProps) {
|
||||
const { t } = useTranslation(["skills", "common"]);
|
||||
const projects = useProjectStore((state) => state.projects);
|
||||
const projects = useProjectStore(selectProjects);
|
||||
const [search, setSearch] = useState("");
|
||||
const [activeFilter, setActiveFilter] = useState<SkillsFilter>("all");
|
||||
const [dialogOpen, setDialogOpen] = useState(false);
|
||||
|
||||
@@ -506,7 +506,7 @@ function handleShared(sessionId: string, update: SessionUpdate): void {
|
||||
currentModelId;
|
||||
|
||||
const sessionStore = useChatSessionStore.getState();
|
||||
sessionStore.updateSession(sessionId, {
|
||||
sessionStore.patchSession(sessionId, {
|
||||
modelId: currentModelId,
|
||||
modelName: currentModelName,
|
||||
});
|
||||
|
||||
@@ -24,7 +24,7 @@ export function handleSessionInfoUpdate(
|
||||
}
|
||||
|
||||
const meta = isRecord(info._meta) ? info._meta : {};
|
||||
const patch: Parameters<typeof sessionStore.updateSession>[1] = {};
|
||||
const patch: Parameters<typeof sessionStore.patchSession>[1] = {};
|
||||
|
||||
if (typeof info.title === "string" && info.title && !session.userSetName) {
|
||||
patch.title = info.title;
|
||||
@@ -40,6 +40,6 @@ export function handleSessionInfoUpdate(
|
||||
}
|
||||
|
||||
if (Object.keys(patch).length > 0) {
|
||||
sessionStore.updateSession(sessionId, patch);
|
||||
sessionStore.patchSession(sessionId, patch);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -119,7 +119,9 @@
|
||||
"filesTitle": "Files"
|
||||
},
|
||||
"notifications": {
|
||||
"compactionComplete": "Conversation compacted. Older context was summarized."
|
||||
"compactionComplete": "Conversation compacted. Older context was summarized.",
|
||||
"moveError": "Failed to move chat",
|
||||
"renameError": "Failed to rename chat"
|
||||
},
|
||||
"message": {
|
||||
"copied": "Copied",
|
||||
|
||||
@@ -119,7 +119,9 @@
|
||||
"filesTitle": "Archivos"
|
||||
},
|
||||
"notifications": {
|
||||
"compactionComplete": "Conversacion compactada. El contexto anterior se resumio."
|
||||
"compactionComplete": "Conversacion compactada. El contexto anterior se resumio.",
|
||||
"moveError": "No se pudo mover el chat",
|
||||
"renameError": "No se pudo cambiar el nombre del chat"
|
||||
},
|
||||
"message": {
|
||||
"copied": "Copiado",
|
||||
|
||||
@@ -1,10 +0,0 @@
|
||||
/** Find the last index in an array matching a predicate. */
|
||||
export function findLastIndex<T>(
|
||||
arr: readonly T[],
|
||||
predicate: (item: T) => boolean,
|
||||
): number {
|
||||
for (let i = arr.length - 1; i >= 0; i--) {
|
||||
if (predicate(arr[i])) return i;
|
||||
}
|
||||
return -1;
|
||||
}
|
||||
+464
@@ -0,0 +1,464 @@
|
||||
**Goose2 Zustand State Management Improvement Plan**
|
||||
|
||||
This plan translates the findings from [goose2-zustand-state-management-review.md](/Users/lifei/Development/goose/ui/goose2/ui_improvements/state_management/goose2-zustand-state-management-review.md) into a concrete implementation sequence.
|
||||
|
||||
The plan is optimized for:
|
||||
- improving maintainability
|
||||
- reducing future bug risk
|
||||
- minimizing unnecessary churn
|
||||
- respecting code dependencies and migration order
|
||||
|
||||
This is a single master plan. It is intentionally sequenced so that higher-leverage, lower-risk changes happen before larger store refactors.
|
||||
|
||||
**Goals**
|
||||
|
||||
- Establish selector-first Zustand usage in React code.
|
||||
- Reduce coupling between consumers and broad store shapes.
|
||||
- Remove hidden backend side effects from generic store actions.
|
||||
- Improve store responsibility boundaries.
|
||||
- Standardize persistence and mutation policy.
|
||||
- Improve test reset discipline and coverage for risky flows.
|
||||
- Only use `useShallow` and Immer where they materially improve maintainability.
|
||||
|
||||
**Sequencing Principles**
|
||||
|
||||
- Fix read patterns before splitting stores.
|
||||
- Separate side effects from generic store actions before redesigning store boundaries.
|
||||
- Split the clearest UI/domain boundaries first.
|
||||
- Standardize persistence only after the intended state boundaries are clearer.
|
||||
- Improve tests alongside each change, not only at the end.
|
||||
- Treat Immer as a late readability improvement, not an architectural fix.
|
||||
|
||||
**High-Level Phase Order**
|
||||
|
||||
1. Remove whole-store subscriptions and stabilize store consumption.
|
||||
2. Introduce a selector-first read layer and add `useShallow` where appropriate.
|
||||
3. Separate backend side effects from generic store actions.
|
||||
4. Split the broadest stores by responsibility.
|
||||
5. Decide chat-session workflow failure policy.
|
||||
6. Refactor `projectStore` into clearer layers.
|
||||
7. Standardize persistence boundaries.
|
||||
8. Standardize store reset/testing patterns and close coverage gaps.
|
||||
9. Optionally adopt Immer for nested update-heavy stores.
|
||||
|
||||
**Priority Snapshot**
|
||||
|
||||
**Highest priority**
|
||||
- Phase 1: whole-store subscriptions
|
||||
- Phase 2: selector-first read layer
|
||||
- Phase 3: hidden backend side effects in `chatSessionStore`
|
||||
|
||||
**Medium priority**
|
||||
- Phase 4: split `agentStore` UI state and `chatSessionStore` UI state
|
||||
- Phase 5: decide chat-session workflow failure policy
|
||||
- Phase 6: refactor `projectStore`
|
||||
- Phase 8: testing/reset cleanup for touched stores
|
||||
|
||||
**Lower priority but useful**
|
||||
- Phase 7: persistence standardization rollout
|
||||
- Phase 9: selective Immer adoption
|
||||
|
||||
**Dependency Logic**
|
||||
|
||||
- Phases 1 and 2 come first because they reduce consumer coupling and make later store splits safer.
|
||||
- Phase 3 comes before major store splits because current action semantics are one of the biggest bug risks.
|
||||
- Phase 4 is safer after read patterns and action semantics are clearer.
|
||||
- Phase 5 follows Phase 4 so archive/unarchive workflow policy can be decided after session-store boundaries are clearer.
|
||||
- Phase 6 depends on lessons from earlier store cleanup and should not be the first major refactor.
|
||||
- Phase 7 depends on clearer decisions about what state is truly durable.
|
||||
- Phase 8 should happen incrementally, but a dedicated cleanup pass is still needed.
|
||||
- Phase 9 is intentionally last because it improves update ergonomics, not architecture.
|
||||
|
||||
**Phase 1: Remove Whole-Store Subscriptions**
|
||||
|
||||
**Goal**
|
||||
- Replace broad bound-store subscriptions with explicit selectors.
|
||||
|
||||
**Why first**
|
||||
- This is the highest-leverage, lowest-risk improvement.
|
||||
- It reduces rerender noise and decouples components/hooks from full store shapes.
|
||||
- It makes later store changes less invasive.
|
||||
|
||||
**Primary files**
|
||||
- [AppShell.tsx](/Users/lifei/Development/goose/ui/goose2/src/app/AppShell.tsx:80)
|
||||
- [Sidebar.tsx](/Users/lifei/Development/goose/ui/goose2/src/features/sidebar/ui/Sidebar.tsx:103)
|
||||
- [usePersonas.ts](/Users/lifei/Development/goose/ui/goose2/src/features/agents/hooks/usePersonas.ts:12)
|
||||
- [useChat.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/hooks/useChat.ts:108)
|
||||
|
||||
**Detailed tasks**
|
||||
- `AppShell.tsx`
|
||||
- Replace `useChatStore()`, `useChatSessionStore()`, `useAgentStore()`, and `useProjectStore()` broad subscriptions with specific selectors.
|
||||
- Avoid selecting whole store objects just to access a few fields or actions.
|
||||
- Keep legitimate `getState()` usage in async helper callbacks where necessary.
|
||||
- `Sidebar.tsx`
|
||||
- Replace whole `chatStore`, `agentStoreState`, and `projectStoreState` subscriptions with targeted selectors.
|
||||
- Avoid broad subscription combined with heavy render-time derivation.
|
||||
- `usePersonas.ts`
|
||||
- Replace `const store = useAgentStore()` with explicit state/action selectors.
|
||||
- Stop relying on the “store object is stable” assumption in hook callbacks.
|
||||
- `useChat.ts`
|
||||
- Replace whole-store subscription with session-scoped selectors for messages/runtime plus specific actions.
|
||||
|
||||
**Success criteria**
|
||||
- No high-level React component or hook uses `useSomeStore()` with no selector.
|
||||
- Components subscribe only to the fields/actions they actually need.
|
||||
|
||||
**Phase 2: Introduce a Selector-First Read Layer**
|
||||
|
||||
**Goal**
|
||||
- Standardize how React code reads Zustand state.
|
||||
|
||||
**Why second**
|
||||
- After Phase 1, repeated selector logic will become visible.
|
||||
- This is the right time to define reusable selectors and add `useShallow` only where needed.
|
||||
|
||||
**Primary files to add or refactor**
|
||||
- `src/features/chat/stores/chatSelectors.ts`
|
||||
- `src/features/chat/stores/chatSessionSelectors.ts`
|
||||
- `src/features/agents/stores/agentSelectors.ts`
|
||||
- `src/features/projects/stores/projectSelectors.ts`
|
||||
- [useProviderInventory.ts](/Users/lifei/Development/goose/ui/goose2/src/features/providers/hooks/useProviderInventory.ts:29)
|
||||
|
||||
**Detailed tasks**
|
||||
- Create selector helpers for repeated reads:
|
||||
- session lists and active session id
|
||||
- session runtime by id
|
||||
- visible messages for session
|
||||
- project list and project lookup helpers
|
||||
- agent/persona/provider simple reads
|
||||
- provider selection state
|
||||
- Keep active session, Home session, active project, and similar composed values derived from existing state. Do not add duplicate store attributes just to make reads easier.
|
||||
- Introduce grouped selectors with `useShallow` where components need multiple values together.
|
||||
- Keep primitive selectors simple and do not wrap them in `useShallow`.
|
||||
- Refactor any repetitive inline lookup logic into selector helpers or pure `lib/` helpers if it is not inherently store-specific.
|
||||
|
||||
**Where `useShallow` should be used**
|
||||
- grouped object selectors in high-level consumers
|
||||
- derived array/object selectors where reference churn would otherwise trigger unnecessary rerenders
|
||||
|
||||
**Where `useShallow` should not be used**
|
||||
- primitive selectors
|
||||
- as a substitute for selector discipline
|
||||
- to compensate for weak store boundaries
|
||||
|
||||
**Success criteria**
|
||||
- Common reads have reusable selector helpers.
|
||||
- `useShallow` appears only on object/array selectors where it adds value.
|
||||
- React read patterns are visibly more consistent across features.
|
||||
|
||||
**Phase 3: Separate Backend Side Effects From Generic Store Actions**
|
||||
|
||||
**Goal**
|
||||
- Make mutation semantics explicit and reduce hidden side effects.
|
||||
|
||||
**Why third**
|
||||
- This is one of the highest bug-risk areas.
|
||||
- It is easier to split stores safely after action semantics are cleaned up.
|
||||
|
||||
**Primary file**
|
||||
- [chatSessionStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatSessionStore.ts:195)
|
||||
|
||||
**Current problem**
|
||||
- `updateSession` looks like a local patch action but can also trigger backend rename/project-update side effects.
|
||||
|
||||
**Detailed tasks**
|
||||
- Split local state patching from backend persistence behavior.
|
||||
- Keep a truly local session patch action in the store.
|
||||
- Move remote mutation behavior into explicit orchestration functions or hooks.
|
||||
- Introduce clearly named operations for:
|
||||
- rename session and persist
|
||||
- update session project and persist
|
||||
- archive session
|
||||
- unarchive session
|
||||
- Audit current call sites that assume `updateSession` is the main entry point:
|
||||
- [useChat.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/hooks/useChat.ts:215)
|
||||
- [useResolvedAgentModelPicker.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/hooks/useResolvedAgentModelPicker.ts:279)
|
||||
- [useChatSessionController.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/hooks/useChatSessionController.ts:314)
|
||||
- [useChatSessionController.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/hooks/useChatSessionController.ts:711)
|
||||
|
||||
**Possible target modules**
|
||||
- `src/features/chat/api/` for backend-facing operations
|
||||
- `src/features/chat/hooks/` for orchestration wrappers
|
||||
|
||||
**Success criteria**
|
||||
- Generic local patch actions are local only.
|
||||
- Backend writes happen through explicitly named orchestration functions.
|
||||
- Mutation semantics are easier to reason about at call sites.
|
||||
|
||||
**Phase 4: Split the Broadest Stores by Responsibility**
|
||||
|
||||
**Goal**
|
||||
- Reduce mixed responsibilities inside stores and isolate UI-only state from domain state where practical.
|
||||
|
||||
**Why fourth**
|
||||
- Consumer coupling and action semantics should be cleaner before changing boundaries.
|
||||
|
||||
**AgentStore Boundary Work**
|
||||
|
||||
**Primary file**
|
||||
- [agentStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/agents/stores/agentStore.ts:34)
|
||||
|
||||
**Current mix**
|
||||
- personas
|
||||
- agents
|
||||
- providers
|
||||
- selected provider
|
||||
- active agent
|
||||
- persona editor modal state
|
||||
|
||||
**Detailed tasks**
|
||||
- Keep catalog/domain concerns together initially:
|
||||
- personas
|
||||
- agents
|
||||
- providers
|
||||
- selected provider
|
||||
- active agent if still truly shared
|
||||
- Move editor/modal concerns out:
|
||||
- `personaEditorOpen`
|
||||
- `editingPersona`
|
||||
- `personaEditorMode`
|
||||
- Update affected consumers:
|
||||
- [AgentsView.tsx](/Users/lifei/Development/goose/ui/goose2/src/features/agents/ui/AgentsView.tsx:44)
|
||||
- [useChatSessionController.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/hooks/useChatSessionController.ts:601)
|
||||
|
||||
**Possible target files**
|
||||
- `agentCatalogStore.ts`
|
||||
- `agentUiStore.ts`
|
||||
|
||||
**ChatSessionStore Boundary Work**
|
||||
|
||||
**Primary file**
|
||||
- [chatSessionStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatSessionStore.ts:55)
|
||||
|
||||
**Current mix**
|
||||
- session records
|
||||
- active session selection
|
||||
- context panel open state
|
||||
- active workspace UI state
|
||||
|
||||
**Detailed tasks**
|
||||
- Keep session data in the session store.
|
||||
- Move UI-only keyed state out:
|
||||
- `contextPanelOpenBySession`
|
||||
- `activeWorkspaceBySession`
|
||||
- Update affected consumers:
|
||||
- [ChatView.tsx](/Users/lifei/Development/goose/ui/goose2/src/features/chat/ui/ChatView.tsx:30)
|
||||
- [ContextPanel.tsx](/Users/lifei/Development/goose/ui/goose2/src/features/chat/ui/ContextPanel.tsx:44)
|
||||
- [useChatSessionController.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/hooks/useChatSessionController.ts:56)
|
||||
- [useChat.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/hooks/useChat.ts:386)
|
||||
|
||||
**Possible target files**
|
||||
- `chatSessionStore.ts`
|
||||
- `chatSessionUiStore.ts`
|
||||
|
||||
**ChatStore Re-Evaluation**
|
||||
|
||||
**Primary file**
|
||||
- [chatStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatStore.ts:39)
|
||||
|
||||
**Current mix**
|
||||
- messages
|
||||
- runtime
|
||||
- queue
|
||||
- drafts
|
||||
- connection state
|
||||
- loading/replay state
|
||||
- scroll targeting
|
||||
- cleanup
|
||||
|
||||
**Detailed tasks**
|
||||
- Do not split this immediately unless the earlier Phase 4 boundary work is already complete.
|
||||
- Reassess whether the app needs a separation between:
|
||||
- message state
|
||||
- runtime state
|
||||
- composer/draft state
|
||||
- Base the split on actual selector usage after cleanup, not on theoretical purity.
|
||||
|
||||
**Success criteria for Phase 4**
|
||||
- UI-only state is no longer co-located with unrelated domain-heavy stores where that separation is clear and useful.
|
||||
- Store boundaries reflect actual shared-state responsibilities more closely.
|
||||
|
||||
**Phase 5: Decide Chat Session Workflow Failure Policy**
|
||||
|
||||
**Goal**
|
||||
- Make chat-session workflow failure behavior explicit after Phase 4 clarifies session-store boundaries.
|
||||
|
||||
**Why this phase matters**
|
||||
- Phase 3 removes hidden side effects from generic session patching.
|
||||
- Phase 4 clarifies which session state should remain in `chatSessionStore`.
|
||||
- Archive/unarchive still have API calls in the store and currently use optimistic-without-rollback behavior.
|
||||
- Failure-policy changes are user-visible, so they should be handled separately from the mechanical side-effect extraction.
|
||||
|
||||
**Detailed tasks**
|
||||
- Review title/project workflows introduced in Phase 3.
|
||||
- Revisit archive/unarchive after Phase 4.
|
||||
- Decide policy per workflow:
|
||||
- backend-first
|
||||
- optimistic without rollback
|
||||
- optimistic with rollback
|
||||
- refresh-on-failure
|
||||
- optimistic with user-visible error
|
||||
- Add focused tests for selected success/failure behavior.
|
||||
|
||||
**Success criteria**
|
||||
- Each chat-session workflow has one clear implementation point.
|
||||
- Local/backend consistency behavior is intentional and tested.
|
||||
|
||||
**Phase 6: Refactor `projectStore` Into Clearer Layers**
|
||||
|
||||
**Goal**
|
||||
- Reduce architectural overreach in the project feature state layer.
|
||||
|
||||
**Why this phase matters**
|
||||
- `projectStore` currently combines state, cache hydration, persistence, CRUD orchestration, and optimistic mutation policy.
|
||||
- That makes project behavior harder to reason about and harder to test than the other feature stores.
|
||||
- This phase should happen after the earlier selector and store-boundary cleanup so the project refactor is not fighting broad consumer coupling at the same time.
|
||||
|
||||
**Primary file**
|
||||
- [projectStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/projects/stores/projectStore.ts:73)
|
||||
|
||||
**Current mix**
|
||||
- project data
|
||||
- loading state
|
||||
- local cache hydration/persistence
|
||||
- CRUD orchestration
|
||||
- optimistic reorder behavior
|
||||
- active selection
|
||||
|
||||
**Detailed tasks**
|
||||
- Keep project state transitions in the store.
|
||||
- Move API orchestration into explicit project commands/hooks.
|
||||
- Make reorder policy explicit.
|
||||
- Decide and document whether reorder is:
|
||||
- optimistic with rollback
|
||||
- optimistic with refresh
|
||||
- pessimistic
|
||||
- Reduce direct persistence coupling in the store so persistence can later be standardized.
|
||||
|
||||
**Likely support files**
|
||||
- `src/features/projects/hooks/useProjectCommands.ts`
|
||||
- `src/features/projects/stores/projectSelectors.ts`
|
||||
|
||||
**Likely consumer files to revisit**
|
||||
- [ProjectsView.tsx](/Users/lifei/Development/goose/ui/goose2/src/features/projects/ui/ProjectsView.tsx:89)
|
||||
- [Sidebar.tsx](/Users/lifei/Development/goose/ui/goose2/src/features/sidebar/ui/Sidebar.tsx:192)
|
||||
- [SettingsModal.tsx](/Users/lifei/Development/goose/ui/goose2/src/features/settings/ui/SettingsModal.tsx:107)
|
||||
|
||||
**Success criteria**
|
||||
- Clearer source-of-truth behavior for projects.
|
||||
- Explicit mutation policy for reorder and CRUD flows.
|
||||
- Less orchestration logic living in the store itself.
|
||||
|
||||
**Phase 7: Standardize Persistence Boundaries**
|
||||
|
||||
**Goal**
|
||||
- Replace ad hoc per-store durability choices with a clearer persistence policy.
|
||||
|
||||
**Why this phase matters**
|
||||
- Persistence choices currently live in scattered store-local helpers and are not governed by a shared rule.
|
||||
- That makes hydration behavior, migration behavior, and “what should survive reloads” decisions inconsistent.
|
||||
- This phase comes after boundary cleanup because persistence should reflect the intended final ownership of state, not the current mixed-responsibility shape.
|
||||
|
||||
**Primary files**
|
||||
- [projectStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/projects/stores/projectStore.ts:11)
|
||||
- [agentStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/agents/stores/agentStore.ts:5)
|
||||
- [draftPersistence.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/draftPersistence.ts:1)
|
||||
|
||||
**Detailed tasks**
|
||||
- Decide what state is truly durable.
|
||||
- Move eligible store-backed durability to Zustand `persist` where appropriate.
|
||||
- Use `partialize` and versioning for persisted stores.
|
||||
- Keep non-store persistence explicit if it truly belongs outside Zustand.
|
||||
|
||||
**Good persistence candidates**
|
||||
- selected provider
|
||||
- chat drafts if intentionally durable
|
||||
- possibly project cache if cached bootstrap remains a product decision
|
||||
|
||||
**Avoid persisting by default**
|
||||
- connection state
|
||||
- transient runtime state
|
||||
- temporary UI interaction state
|
||||
|
||||
**Success criteria**
|
||||
- durability rules are explicit
|
||||
- persisted state is minimal and intentional
|
||||
- persistence code is no longer scattered ad hoc through stores
|
||||
|
||||
**Phase 8: Standardize Test Reset Patterns and Close Coverage Gaps**
|
||||
|
||||
**Goal**
|
||||
- Make store tests safer and add coverage to the riskiest state-management paths.
|
||||
|
||||
**Why this phase matters**
|
||||
- The current tests prove some important store behavior, but reset discipline is inconsistent and some of the riskiest stores are still under-covered.
|
||||
- As the earlier phases change boundaries and mutation semantics, test quality becomes more important, not less.
|
||||
- This phase ensures the refactor leaves the state layer easier to trust and easier to evolve.
|
||||
|
||||
**Primary files**
|
||||
- [agentStore.test.ts](/Users/lifei/Development/goose/ui/goose2/src/features/agents/stores/__tests__/agentStore.test.ts:37)
|
||||
- [usePersonas.test.ts](/Users/lifei/Development/goose/ui/goose2/src/features/agents/hooks/__tests__/usePersonas.test.ts:53)
|
||||
- [chatStore.test.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/__tests__/chatStore.test.ts:22)
|
||||
- additional chat hook tests that use partial `setState(...)` setup patterns
|
||||
|
||||
**Detailed tasks**
|
||||
- Introduce initial-state reset helpers for each store.
|
||||
- Stop relying on partial shallow-merge resets as the standard setup pattern.
|
||||
- Apply the same reset discipline to hook tests that manipulate stores directly.
|
||||
|
||||
**Coverage gaps to address**
|
||||
- `projectStore`
|
||||
- cache hydration
|
||||
- reorder semantics
|
||||
- mutation failure/reconciliation policy
|
||||
- `providerInventoryStore`
|
||||
- merge semantics
|
||||
- loading behavior
|
||||
- session mutation orchestration introduced in Phase 3
|
||||
|
||||
**Success criteria**
|
||||
- tests reset stores from a known initial state
|
||||
- risky persistence and mutation behaviors have direct coverage
|
||||
|
||||
**Phase 9: Optional Immer Adoption for Nested Update Ergonomics**
|
||||
|
||||
**Goal**
|
||||
- Improve readability where nested immutable updates remain noisy after structural cleanup.
|
||||
|
||||
**Why this phase matters**
|
||||
- Immer is not a structural fix, so it should not be introduced before the higher-value boundary and side-effect problems are addressed.
|
||||
- After the earlier phases, it becomes much easier to judge whether nested update boilerplate is still a real maintainability cost.
|
||||
- Keeping this phase late prevents the team from mistaking update ergonomics for architectural improvement.
|
||||
|
||||
**Primary candidate**
|
||||
- [chatStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatStore.ts:104)
|
||||
|
||||
**Secondary candidate**
|
||||
- [chatSessionStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatSessionStore.ts:137)
|
||||
|
||||
**Detailed tasks**
|
||||
- Reassess nested update-heavy store actions after earlier phases.
|
||||
- Only adopt Immer if update readability remains a real pain point.
|
||||
- Do not use Immer as a justification to keep a broad or mixed-responsibility store intact.
|
||||
|
||||
**Success criteria**
|
||||
- update logic is easier to read without obscuring ownership boundaries
|
||||
|
||||
**Suggested PR Breakdown**
|
||||
|
||||
1. PR 1: selector cleanup in `AppShell`, `Sidebar`, `usePersonas`, `useChat`
|
||||
2. PR 2: selector helpers + selective `useShallow`
|
||||
3. PR 3: split `chatSessionStore` local patching from backend side effects
|
||||
4. PR 4: extract `agentUiStore` from `agentStore`
|
||||
5. PR 5: extract `chatSessionUiStore` from `chatSessionStore`
|
||||
6. PR 6: refactor `projectStore` orchestration and mutation policy
|
||||
7. PR 7: persistence standardization
|
||||
8. PR 8: reset helpers + coverage expansion
|
||||
9. PR 9: optional Immer cleanup for nested update-heavy stores
|
||||
|
||||
**Plan Usage**
|
||||
|
||||
- Use this as the master sequencing document.
|
||||
- Do not try to execute every phase in a single refactor.
|
||||
- For implementation, create smaller phase-specific execution notes only when a phase is about to start.
|
||||
- Re-check the plan after Phase 3 and again after Phase 6, because store boundaries and actual consumer patterns may change enough to adjust later phases.
|
||||
@@ -0,0 +1,384 @@
|
||||
**Goose2 Zustand Review: Code Quality + Architecture**
|
||||
|
||||
If the bar is “easy to maintain later” and “low future bug surface,” the current Zustand usage in `ui/goose2` does **not** meet that bar yet.
|
||||
|
||||
This review combines two perspectives:
|
||||
- strict Zustand usage and code-quality review
|
||||
- higher-level architectural review of how Zustand is being used in the frontend
|
||||
|
||||
The conclusions are based on reading the current `ui/goose2` stores, major consumers, and store tests.
|
||||
|
||||
Primary reference points from the official Zustand docs:
|
||||
- Intro and selector usage: https://zustand.docs.pmnd.rs/getting-started/introduction
|
||||
- Flux-inspired practice: https://zustand.docs.pmnd.rs/learn/guides/flux-inspired-practice
|
||||
- Slices pattern: https://zustand.docs.pmnd.rs/learn/guides/slices-pattern
|
||||
- Auto-generating selectors: https://zustand.docs.pmnd.rs/learn/guides/auto-generating-selectors
|
||||
- `useShallow`: https://zustand.docs.pmnd.rs/learn/guides/prevent-rerenders-with-use-shallow
|
||||
- `persist`: https://zustand.docs.pmnd.rs/reference/middlewares/persist
|
||||
- Reset pattern: https://zustand.docs.pmnd.rs/learn/guides/how-to-reset-state
|
||||
- `Map` / `Set` usage: https://zustand.docs.pmnd.rs/learn/guides/maps-and-sets-usage
|
||||
|
||||
**Overall Verdict**
|
||||
|
||||
`ui/goose2` uses Zustand well enough to ship features, but not strictly enough for long-term maintainability.
|
||||
|
||||
The biggest problems are:
|
||||
1. broad subscriptions to entire stores in React
|
||||
2. stores with weak responsibility boundaries
|
||||
3. state stores doubling as workflow and backend side-effect layers
|
||||
4. ad hoc persistence and inconsistent durability rules
|
||||
5. insufficiently explicit mutation/reconciliation policy
|
||||
6. inconsistent testing/reset discipline
|
||||
|
||||
This is not a case of “Zustand is wrong.” It is a case of “the current architecture makes Zustand too easy to use as a global bucket for state, persistence, and orchestration.”
|
||||
|
||||
**What Is Good**
|
||||
|
||||
- State updates mostly go through `set` and are implemented immutably.
|
||||
- `Map` and `Set` usage is correct where present, because updates create new instances. Examples: [providerInventoryStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/providers/stores/providerInventoryStore.ts:19), [chatStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatStore.ts:452).
|
||||
- Some consumers already use narrow selectors correctly, for example [useProviderSelection.ts](/Users/lifei/Development/goose/ui/goose2/src/features/agents/hooks/useProviderSelection.ts:1).
|
||||
- `getState()` usage in non-React async/event code is acceptable and not a problem by itself.
|
||||
- Important stores like chat and agent stores already have meaningful tests.
|
||||
|
||||
**Detailed Findings**
|
||||
|
||||
1. **Whole-store subscriptions are the clearest Zustand misuse**
|
||||
|
||||
In React code, bound store hooks should not be called without selectors.
|
||||
|
||||
Concrete examples:
|
||||
- [AppShell.tsx](/Users/lifei/Development/goose/ui/goose2/src/app/AppShell.tsx:80)
|
||||
- [Sidebar.tsx](/Users/lifei/Development/goose/ui/goose2/src/features/sidebar/ui/Sidebar.tsx:103)
|
||||
- [Sidebar.tsx](/Users/lifei/Development/goose/ui/goose2/src/features/sidebar/ui/Sidebar.tsx:191)
|
||||
- [usePersonas.ts](/Users/lifei/Development/goose/ui/goose2/src/features/agents/hooks/usePersonas.ts:12)
|
||||
- [useChat.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/hooks/useChat.ts:108)
|
||||
|
||||
Why this is a problem:
|
||||
- unrelated store updates can rerender large components/hooks
|
||||
- consumers become tightly coupled to the full store shape
|
||||
- store refactors become expensive because consumers depend on broad internals
|
||||
|
||||
Strict standard:
|
||||
- no `useSomeStore()` without a selector in React components or hooks
|
||||
|
||||
2. **`useShallow` is underused where it matters**
|
||||
|
||||
There is effectively no `useShallow` usage in the `src/` tree.
|
||||
|
||||
That is not the primary problem, but it is a real gap after selector discipline is fixed.
|
||||
|
||||
Where it would matter:
|
||||
- grouped selectors returning objects
|
||||
- computed selectors returning arrays/objects
|
||||
- high-level components that need a few store fields together
|
||||
|
||||
Good target pattern:
|
||||
```ts
|
||||
const { sessions, activeSessionId } = useChatSessionStore(
|
||||
useShallow((s) => ({
|
||||
sessions: s.sessions,
|
||||
activeSessionId: s.activeSessionId,
|
||||
})),
|
||||
);
|
||||
```
|
||||
|
||||
Strict standard:
|
||||
- first adopt selector-first consumption
|
||||
- then use `useShallow` on object/array selectors
|
||||
- do not use `useShallow` on primitive selectors
|
||||
|
||||
More specifically, `useShallow` is good for:
|
||||
- replacing whole-store subscriptions with grouped object selectors in high-level consumers such as [AppShell.tsx](/Users/lifei/Development/goose/ui/goose2/src/app/AppShell.tsx:80), [Sidebar.tsx](/Users/lifei/Development/goose/ui/goose2/src/features/sidebar/ui/Sidebar.tsx:103), [usePersonas.ts](/Users/lifei/Development/goose/ui/goose2/src/features/agents/hooks/usePersonas.ts:12), and [useChat.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/hooks/useChat.ts:108)
|
||||
- derived object/array selectors in places like [useProviderInventory.ts](/Users/lifei/Development/goose/ui/goose2/src/features/providers/hooks/useProviderInventory.ts:29)
|
||||
|
||||
`useShallow` is not for:
|
||||
- primitive selectors like `loading`, `activeSessionId`, or `selectedProvider`
|
||||
- compensating for weak store boundaries or hidden store side effects
|
||||
- replacing selector discipline with a blanket optimization
|
||||
|
||||
Maintainability conclusion:
|
||||
- `useShallow` is a good tactical improvement after selector cleanup
|
||||
- it does not replace the higher-value fixes in this review: selector-first reads, clearer store boundaries, explicit side-effect boundaries, and standardized persistence/reconciliation rules
|
||||
|
||||
3. **Stores are too broad in responsibility**
|
||||
|
||||
Separate stores are not automatically wrong, but several individual stores are carrying too many concerns.
|
||||
|
||||
Examples:
|
||||
- [chatStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatStore.ts:39)
|
||||
- [agentStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/agents/stores/agentStore.ts:34)
|
||||
- [chatSessionStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatSessionStore.ts:55)
|
||||
- [projectStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/projects/stores/projectStore.ts:37)
|
||||
|
||||
What that looks like:
|
||||
- `chatStore` mixes messages, runtime, drafts, queue, connection state, loading/replay state, scroll targeting, and cleanup
|
||||
- `agentStore` mixes personas, agents, providers, selected provider, active agent, and persona editor UI state
|
||||
- `chatSessionStore` mixes session records, active session selection, context-panel state, and workspace UI state
|
||||
- `projectStore` mixes project records, loading state, persistence, CRUD orchestration, optimistic reordering, and active selection
|
||||
|
||||
Why this is a problem:
|
||||
- unrelated state changes share subscriber surfaces
|
||||
- tests become broader than necessary
|
||||
- small changes require too much knowledge of one store’s internals
|
||||
|
||||
Immer is relevant here only as an update-readability tool, not as a boundary fix. It may improve maintainability in nested update-heavy stores such as [chatStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatStore.ts:104), where updates to `messagesBySession`, `sessionStateById`, `draftsBySession`, and `scrollTargetMessageBySession` currently require substantial object spread boilerplate, and to a lesser extent in [chatSessionStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatSessionStore.ts:137). It would not solve the higher-value problems in this review: broad store responsibilities, workflow/state mixing, hidden backend side effects, or ad hoc persistence. It is also unlikely to add much value to already-flat stores like [providerInventoryStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/providers/stores/providerInventoryStore.ts:19).
|
||||
|
||||
4. **Domain state and UI state are mixed together**
|
||||
|
||||
This is one of the clearest architectural problems.
|
||||
|
||||
Examples:
|
||||
- [agentStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/agents/stores/agentStore.ts:56)
|
||||
- [chatSessionStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatSessionStore.ts:60)
|
||||
|
||||
Specifically:
|
||||
- `agentStore` holds persona/agent/provider data and also persona editor modal state
|
||||
- `chatSessionStore` holds session data and also context panel open state plus per-session workspace UI state
|
||||
|
||||
Why this is a problem:
|
||||
- UI interaction state and domain data are forced into the same lifecycle
|
||||
- unrelated UI churn can affect store consumers interested in durable feature data
|
||||
- state boundaries are harder to reason about
|
||||
|
||||
Strict standard:
|
||||
- if state is only for one screen or interaction, prefer local state or a focused UI store
|
||||
- if state is shared across views or sessions, use Zustand
|
||||
- do not default to putting modal/open-state UI into domain-heavy stores
|
||||
|
||||
5. **Store actions hide backend and persistence side effects**
|
||||
|
||||
The clearest example is `chatSessionStore.updateSession`.
|
||||
|
||||
Code:
|
||||
- [chatSessionStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatSessionStore.ts:195)
|
||||
- [chatSessionStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatSessionStore.ts:211)
|
||||
- [chatSessionStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatSessionStore.ts:224)
|
||||
|
||||
Why this is a problem:
|
||||
- `updateSession` sounds like a local patch, but it may also rename a backend session or update the backend project association
|
||||
- action names stop being truthful about side effects
|
||||
- rollback, retry, and reconciliation responsibilities become unclear
|
||||
|
||||
Strict standard:
|
||||
- local patch actions should be local only
|
||||
- side-effectful operations should be explicit, for example `renameSessionAndPersist`
|
||||
- transport and orchestration should stay in `api/` modules or dedicated hooks/commands
|
||||
|
||||
6. **Zustand is being used as a workflow layer, not just a state layer**
|
||||
|
||||
This is the main architectural synthesis.
|
||||
|
||||
Examples:
|
||||
- [projectStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/projects/stores/projectStore.ts:73)
|
||||
- [chatSessionStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatSessionStore.ts:195)
|
||||
- [chatStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatStore.ts:415)
|
||||
|
||||
What to look for:
|
||||
- local storage helpers embedded in stores
|
||||
- API calls inside store actions
|
||||
- optimistic update behavior living in the store
|
||||
- store methods that are really workflow/orchestration entry points
|
||||
|
||||
Architectural conclusion:
|
||||
- Zustand should primarily own shared client state
|
||||
- hooks and command-style modules should own workflow/orchestration
|
||||
- `api/` should own backend transport
|
||||
|
||||
7. **Persistence is hand-rolled where `persist` would provide a cleaner boundary**
|
||||
|
||||
Examples:
|
||||
- [projectStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/projects/stores/projectStore.ts:11)
|
||||
- [agentStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/agents/stores/agentStore.ts:5)
|
||||
- [chatStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatStore.ts:18)
|
||||
|
||||
Why this is a problem:
|
||||
- repeated `localStorage` code
|
||||
- each store invents its own hydration semantics
|
||||
- no versioning, migrations, or `partialize`
|
||||
- persistence rules are not centralized or explicit
|
||||
|
||||
Architectural conclusion:
|
||||
- durability should be a standard boundary, not a per-store ad hoc decision
|
||||
|
||||
8. **Selectors are not a stable architectural read API yet**
|
||||
|
||||
The codebase often relies on imperative helper methods instead of a selector-oriented read layer.
|
||||
|
||||
Examples:
|
||||
- [agentStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/agents/stores/agentStore.ts:188)
|
||||
- [agentStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/agents/stores/agentStore.ts:213)
|
||||
- [projectStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/projects/stores/projectStore.ts:181)
|
||||
- [chatStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatStore.ts:184)
|
||||
- [chatStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatStore.ts:191)
|
||||
- [chatSessionStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatSessionStore.ts:329)
|
||||
|
||||
These helpers are acceptable for `getState()` usage in non-React code, but in React they push the app toward imperative reads instead of reactive selectors.
|
||||
|
||||
Architectural conclusion:
|
||||
- the app needs a selector-first read API
|
||||
- common selectors should be promoted into helper hooks
|
||||
- React components should not depend on imperative store helper methods by default
|
||||
|
||||
9. **Some render paths perform heavy derived work while broadly subscribed**
|
||||
|
||||
The clearest example is the sidebar.
|
||||
|
||||
Code:
|
||||
- [Sidebar.tsx](/Users/lifei/Development/goose/ui/goose2/src/features/sidebar/ui/Sidebar.tsx:103)
|
||||
- [Sidebar.tsx](/Users/lifei/Development/goose/ui/goose2/src/features/sidebar/ui/Sidebar.tsx:145)
|
||||
- [Sidebar.tsx](/Users/lifei/Development/goose/ui/goose2/src/features/sidebar/ui/Sidebar.tsx:194)
|
||||
|
||||
Why this is a problem:
|
||||
- broad subscription plus per-render grouping/sorting/runtime lookups compounds rerender cost
|
||||
- derivation logic is harder to isolate, test, and reuse
|
||||
|
||||
Architectural conclusion:
|
||||
- central derived state patterns should live in selector helpers or pure `lib/` utilities
|
||||
- avoid render-time traversal of large store structures when the same logic is core to the feature
|
||||
|
||||
10. **`projectStore` is the strongest example of architectural overreach**
|
||||
|
||||
Code:
|
||||
- [projectStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/projects/stores/projectStore.ts:73)
|
||||
- [projectStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/projects/stores/projectStore.ts:78)
|
||||
- [projectStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/projects/stores/projectStore.ts:157)
|
||||
|
||||
It currently acts as:
|
||||
- project state store
|
||||
- local cache owner
|
||||
- CRUD orchestration layer
|
||||
- optimistic reorder owner
|
||||
- active selection owner
|
||||
|
||||
Why this is a problem:
|
||||
- one module owns too many failure modes
|
||||
- source-of-truth rules are muddy
|
||||
- harder to add retries, rollback, or refresh policies later
|
||||
|
||||
11. **Async mutation and reconciliation policy is not explicit enough**
|
||||
|
||||
Examples:
|
||||
- [projectStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/projects/stores/projectStore.ts:157)
|
||||
- [chatSessionStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatSessionStore.ts:251)
|
||||
|
||||
What to look for:
|
||||
- local state updated first
|
||||
- backend call later
|
||||
- failures mostly logged
|
||||
- no explicit rollback or refresh strategy encoded in the abstraction
|
||||
|
||||
Architectural conclusion:
|
||||
- each mutation should clearly be one of:
|
||||
- pessimistic
|
||||
- optimistic with rollback
|
||||
- optimistic with guaranteed refresh
|
||||
- that policy should be explicit and consistent
|
||||
|
||||
12. **Test reset discipline is not strict enough**
|
||||
|
||||
Examples:
|
||||
- [agentStore.test.ts](/Users/lifei/Development/goose/ui/goose2/src/features/agents/stores/__tests__/agentStore.test.ts:37)
|
||||
- [usePersonas.test.ts](/Users/lifei/Development/goose/ui/goose2/src/features/agents/hooks/__tests__/usePersonas.test.ts:53)
|
||||
- [chatStore.test.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/__tests__/chatStore.test.ts:22)
|
||||
|
||||
Why this is a problem:
|
||||
- `setState` shallow-merges
|
||||
- tests sometimes omit fields when resetting state
|
||||
- stale state can leak across tests
|
||||
|
||||
Strict standard:
|
||||
- reset each store from its initial state or `getInitialState()` pattern
|
||||
|
||||
13. **Coverage is uneven for side-effectful stores**
|
||||
|
||||
Well-covered:
|
||||
- [agentStore.test.ts](/Users/lifei/Development/goose/ui/goose2/src/features/agents/stores/__tests__/agentStore.test.ts:1)
|
||||
- [chatStore.test.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/__tests__/chatStore.test.ts:1)
|
||||
- [chatSessionStore.test.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/__tests__/chatSessionStore.test.ts:1)
|
||||
|
||||
Noticeably under-covered:
|
||||
- [projectStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/projects/stores/projectStore.ts:1)
|
||||
- [providerInventoryStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/providers/stores/providerInventoryStore.ts:1)
|
||||
|
||||
Missing high-value coverage areas:
|
||||
- project cache hydration
|
||||
- reorder semantics
|
||||
- optimistic failure policy
|
||||
- inventory merge semantics
|
||||
|
||||
14. **There are no clear house rules for what belongs in Zustand**
|
||||
|
||||
This is the meta architectural issue.
|
||||
|
||||
Code evidence comes from inconsistency across modules:
|
||||
- broad whole-store consumer: [AppShell.tsx](/Users/lifei/Development/goose/ui/goose2/src/app/AppShell.tsx:80)
|
||||
- narrow selector consumer: [useProviderSelection.ts](/Users/lifei/Development/goose/ui/goose2/src/features/agents/hooks/useProviderSelection.ts:1)
|
||||
- store-embedded persistence: [projectStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/projects/stores/projectStore.ts:11)
|
||||
- store-embedded backend side effects: [chatSessionStore.ts](/Users/lifei/Development/goose/ui/goose2/src/features/chat/stores/chatSessionStore.ts:195)
|
||||
- local UI state still handled in component: [AppShell.tsx](/Users/lifei/Development/goose/ui/goose2/src/app/AppShell.tsx:58)
|
||||
|
||||
What this means:
|
||||
- the codebase does not yet have a shared rule set for:
|
||||
- local vs shared state
|
||||
- ephemeral vs durable state
|
||||
- store state vs orchestration logic
|
||||
- selector API expectations
|
||||
- persistence boundaries
|
||||
|
||||
**Clarifications So We Don’t Overstate**
|
||||
|
||||
- Multiple separate stores are not automatically wrong.
|
||||
- `Map` and `Set` are not wrong here; they are used correctly.
|
||||
- `getState()` is not wrong in non-React async/event code.
|
||||
- Colocating actions with state is a recommended Zustand pattern. The issue is not action location by itself, but unclear responsibilities and hidden side effects.
|
||||
- The architecture recommendations below are a synthesis of the current findings. They are not all direct “code defects,” but they are the most defensible target design responses to the issues in the current code.
|
||||
|
||||
**Architectural Target Direction**
|
||||
|
||||
The clean target architecture is:
|
||||
- React local state for local view concerns
|
||||
- Zustand for shared client state needed by multiple consumers
|
||||
- `api/` modules for backend transport
|
||||
- hooks or command-style modules for orchestration and reconciliation
|
||||
- pure `lib/` utilities for derivation and transformation
|
||||
|
||||
Zustand should be a small, predictable shared-state layer, not a catch-all workflow layer.
|
||||
|
||||
A healthier direction would be to separate concerns more explicitly, for example:
|
||||
- session data state vs session UI state
|
||||
- agent/provider catalog state vs editor/modal UI state
|
||||
- chat message state vs chat composer/runtime state
|
||||
- workflow commands/hooks for backend mutations and reconciliation
|
||||
|
||||
This exact split does not need to be implemented all at once, but the codebase should move in that direction.
|
||||
|
||||
**Recommended Standard For `ui/goose2`**
|
||||
|
||||
- Never call a bound Zustand hook without a selector in React components or hooks.
|
||||
- Use `useShallow` only when a selector returns an object or array.
|
||||
- Keep store actions either:
|
||||
- pure local state transitions, or
|
||||
- explicitly named async commands with clear side-effect semantics.
|
||||
- Do not mix domain data state and modal/open-state UI unless truly necessary.
|
||||
- Use `persist` for durable state, with `partialize` and `version`.
|
||||
- Prefer selector hooks/helpers for React reads.
|
||||
- Use `getState()` mainly in non-React async/event code.
|
||||
- Reset stores in tests from initial state, not partial merge patches.
|
||||
- When a store grows large, split it by responsibility or by slices.
|
||||
- For every backend mutation, make the optimistic/pessimistic/reconciliation policy explicit.
|
||||
- Establish a team rule for what belongs in Zustand vs local state vs hooks vs `api/`.
|
||||
|
||||
**Bottom Line**
|
||||
|
||||
Strictly judged against Zustand good practices and from an architectural perspective, `ui/goose2` is **adequate for shipping features quickly, but not disciplined enough for long-term maintainability**.
|
||||
|
||||
The highest-value issues to fix are:
|
||||
1. whole-store subscriptions
|
||||
2. broad mixed-responsibility stores
|
||||
3. domain state and UI state mixed in the same stores
|
||||
4. hidden backend side effects in generic store actions
|
||||
5. Zustand being used as a workflow layer instead of primarily a state layer
|
||||
6. ad hoc persistence
|
||||
7. weak selector and test-reset discipline
|
||||
|
||||
These are the areas most likely to create future bugs and make the code harder to refactor, reason about, and test.
|
||||
@@ -0,0 +1,61 @@
|
||||
**Phase 1: Remove Whole-Store Subscriptions**
|
||||
|
||||
**Status**
|
||||
- Not started.
|
||||
|
||||
**Goal**
|
||||
- Replace broad bound-store subscriptions with explicit selectors.
|
||||
- Keep behavior unchanged.
|
||||
- Reduce coupling before any store boundary changes.
|
||||
|
||||
**Scope**
|
||||
- `ui/goose2/src/app/AppShell.tsx`
|
||||
- `ui/goose2/src/features/sidebar/ui/Sidebar.tsx`
|
||||
- `ui/goose2/src/features/agents/hooks/usePersonas.ts`
|
||||
- `ui/goose2/src/features/chat/hooks/useChat.ts`
|
||||
|
||||
**Out Of Scope**
|
||||
- Do not split stores.
|
||||
- Do not move backend API calls.
|
||||
- Do not introduce Zustand `persist`.
|
||||
- Do not introduce Immer.
|
||||
- Do not create a broad selector framework unless repeated selectors become obvious.
|
||||
|
||||
**Execution Steps**
|
||||
|
||||
1. Establish the current baseline.
|
||||
- Run `rg "use[A-Za-z0-9]+Store\\(\\)" ui/goose2/src`.
|
||||
- Confirm broad subscriptions are limited to the known Phase 1 targets or document any additional targets.
|
||||
|
||||
2. Refactor `AppShell.tsx`.
|
||||
- Replace `useChatStore()` with selectors for only the used fields/actions.
|
||||
- Replace `useChatSessionStore()` with selectors for only the used fields/actions.
|
||||
- Replace `useAgentStore()` with selectors for only the used fields/actions.
|
||||
- Replace `useProjectStore()` with selectors for only the used fields/actions.
|
||||
- Keep `useStore.getState()` inside async callbacks where the code intentionally needs latest state at call time.
|
||||
|
||||
3. Refactor `Sidebar.tsx`.
|
||||
- Replace `useChatStore()` with selectors for `messagesBySession` and any action/helper actually needed during render.
|
||||
- Replace `useChatSessionStore()` broad destructuring with selectors.
|
||||
- Replace `useAgentStore()` and `useProjectStore()` broad reads with targeted selectors or narrow `getState()` usage in callbacks.
|
||||
- Keep render output and sorting behavior unchanged.
|
||||
|
||||
4. Refactor `usePersonas.ts`.
|
||||
- Replace `const store = useAgentStore()` with selected actions/state.
|
||||
- Use selected actions in callbacks.
|
||||
- Avoid adding dependencies that recreate timers unnecessarily.
|
||||
- If a callback needs latest store state rather than reactive state, use `useAgentStore.getState()` explicitly.
|
||||
|
||||
5. Refactor `useChat.ts`.
|
||||
- Replace `const store = useChatStore()` with selectors for session-specific messages/runtime and selected actions.
|
||||
- Preserve existing use of `getState()` where callbacks intentionally read current runtime state.
|
||||
- Avoid changing prompt, queue, compaction, or message mutation semantics.
|
||||
|
||||
**Validation**
|
||||
- `rg "use[A-Za-z0-9]+Store\\(\\)" ui/goose2/src`
|
||||
- `cd ui/goose2 && pnpm test -- useChat usePersonas Sidebar`
|
||||
|
||||
**Success Criteria**
|
||||
- No high-level React component or hook in scope calls a bound Zustand hook without a selector.
|
||||
- Behavior remains unchanged.
|
||||
- No store boundary or persistence changes are included.
|
||||
@@ -0,0 +1,95 @@
|
||||
**Phase 2: Introduce A Selector-First Read Layer**
|
||||
|
||||
**Status**
|
||||
- Complete.
|
||||
|
||||
**Goal**
|
||||
- Standardize repeated Zustand reads after Phase 1 exposes selector duplication.
|
||||
- Add `useShallow` only where object or array selector results benefit from shallow comparison.
|
||||
- Use selectors to reveal state boundaries, not hide weak store or component boundaries.
|
||||
- Treat selector files as the current read-layer map, not as a final architecture. Later phases must revisit selector files when state moves across store boundaries.
|
||||
|
||||
**Scope**
|
||||
- `ui/goose2/src/features/chat/stores/chatSelectors.ts`
|
||||
- `ui/goose2/src/features/chat/stores/chatSessionSelectors.ts`
|
||||
- `ui/goose2/src/features/agents/stores/agentSelectors.ts`
|
||||
- `ui/goose2/src/features/projects/stores/projectSelectors.ts`
|
||||
- `ui/goose2/src/features/providers/hooks/useProviderInventory.ts`
|
||||
- Any Phase 1 call sites that now duplicate selector logic.
|
||||
|
||||
**Out Of Scope**
|
||||
- Do not split stores.
|
||||
- Do not change action semantics.
|
||||
- Do not move backend calls.
|
||||
- Do not add `useShallow` around primitive selectors.
|
||||
- Do not introduce selectors for one-off reads unless they improve clarity.
|
||||
- Do not create selector files mechanically for every store field.
|
||||
- Do not add stored attributes for values that can be safely derived.
|
||||
|
||||
**Execution Steps**
|
||||
|
||||
1. Review and classify selectors from Phase 1.
|
||||
- Look for repeated active session, active project, active agent, message, runtime, and provider-selection reads.
|
||||
- Classify each candidate as one of:
|
||||
- common simple read
|
||||
- derived value
|
||||
- symptom of a store with too many responsibilities
|
||||
- symptom of a component doing too much orchestration
|
||||
- one-off inline selector
|
||||
- Keep one-off selectors inline unless abstraction clearly improves readability.
|
||||
- Record store-boundary and component-boundary issues as follow-ups instead of hiding them behind selectors.
|
||||
|
||||
2. Add selector helpers only for common simple reads.
|
||||
- Start with pure selector functions.
|
||||
- Prefer names that describe the value, not the component that consumes it.
|
||||
- Keep selector modules close to the store they read.
|
||||
- When a later phase moves state out of a store, move or delete the related selectors in the same phase so selector files do not preserve old boundaries by accident.
|
||||
- Selector fallbacks must be stable. Do not return fresh `[]` or `{}` values directly from selectors; use module-level constants or derive outside the selector.
|
||||
- Good candidates:
|
||||
- chat: `selectMessagesBySession`, `selectSessionStateById`
|
||||
- chat sessions: `selectSessions`, `selectActiveSessionId`, `selectHasHydratedSessions`, `selectSessionsLoading`
|
||||
- agents: `selectPersonas`, `selectPersonasLoading`, `selectSelectedProvider`
|
||||
- projects: `selectProjects`, `selectProjectsLoading`
|
||||
|
||||
3. Prefer pure derived helpers for derived values.
|
||||
- Active session should be derived from `sessions + activeSessionId`; do not store a second `activeSession` attribute unless there is a strong reason.
|
||||
- Project lookup by id can be a pure helper over `projects`.
|
||||
- Sidebar session grouping may belong in a pure helper or dedicated selector-like utility, but should be designed carefully because it combines sessions, messages, runtime state, and project ids.
|
||||
- Sidebar: evaluate whether derived sidebar session items should be produced by selector/helper code instead of selecting the whole `sessionStateById` record during render.
|
||||
- `AppShell` active/home session derivation should remain derived from selected session state for now; do not add duplicate store attributes.
|
||||
|
||||
4. Revisit hooks that mix read state and command functions.
|
||||
- `usePersonas()` currently returns `personas` and `isLoading` as well as command functions.
|
||||
- `AgentsView` already reads `personas` and `personasLoading` directly with store selectors and only uses `usePersonas()` for command functions.
|
||||
- Decide whether to keep the current contract or split toward a clearer read-selector plus command-hook pattern.
|
||||
- Do not make this contract change in Phase 1.
|
||||
|
||||
5. Introduce `useShallow` selectively.
|
||||
- Use it for grouped object selectors in high-level consumers.
|
||||
- Use it for derived array/object selectors where reference churn causes unnecessary rerenders.
|
||||
- Do not use it for primitive values or single function selectors.
|
||||
- Do not use `useShallow` to compensate for a broad store or overly broad component dependency.
|
||||
- Re-check `useProviderInventory` during this phase. It currently reads `entries` and `loading` separately; only group them with `useShallow` if that improves clarity without introducing object churn.
|
||||
- Phase 2 assessment: keep `useProviderInventory` as separate `entries` and `loading` selectors for now. Grouping them with `useShallow` would mostly reduce two store subscriptions to one, but would not narrow rerenders or simplify consumers.
|
||||
|
||||
6. Refactor consumers to use selector helpers where they remove duplication.
|
||||
- Keep call sites readable.
|
||||
- Avoid turning every selector into an abstraction.
|
||||
- If a consumer needs many selectors, decide whether that indicates a later component decomposition or store-boundary task.
|
||||
- Start with small chat-session selector helpers in `AppShell.tsx` and `Sidebar.tsx`; do not refactor every selector candidate at once.
|
||||
- Record `AppShell` as a later component-decomposition candidate rather than trying to hide its orchestration breadth behind selector helpers.
|
||||
- Record `Sidebar` derived session grouping as a possible pure helper extraction, not as store state.
|
||||
- For `agentStore`, add selectors only for stable repeated domain reads such as personas. Do not add selector helpers for persona editor UI state in this phase, because that state is a Phase 4 extraction candidate.
|
||||
- If selector extraction makes a feature look syntactically cleaner but still leaves many store dependencies, record that as a component or store boundary problem instead of adding more selectors.
|
||||
- Before closing Phase 2, list selector files created and the later phases that must revisit them.
|
||||
|
||||
**Validation**
|
||||
- `rg "useShallow|zustand/shallow" ui/goose2/src`
|
||||
- `rg "use[A-Za-z0-9]+Store\\(\\)" ui/goose2/src`
|
||||
- `cd ui/goose2 && pnpm test -- useChat usePersonas Sidebar useProviderInventory`
|
||||
|
||||
**Success Criteria**
|
||||
- Common read patterns have reusable selector helpers.
|
||||
- `useShallow` appears only on object or array selectors where it adds value.
|
||||
- React store consumption is more consistent without changing behavior.
|
||||
- Phase 3-5 follow-ups are clearer because selector audit findings distinguish simple read cleanup from deeper architecture problems.
|
||||
@@ -0,0 +1,96 @@
|
||||
**Phase 3: Separate Chat Session Side Effects From Generic Store Actions**
|
||||
|
||||
**Status**
|
||||
- Complete.
|
||||
|
||||
**Goal**
|
||||
- Make session mutation semantics explicit.
|
||||
- Ensure generic local patch actions do not hide backend writes.
|
||||
- Use backend-first operations for persisted session fields so local state is patched only after backend success.
|
||||
- Keep this phase focused on side-effect semantics even though Phase 2 identified broader `chatSessionStore` responsibility overload.
|
||||
- Create clearly named operation functions so backend failure policy is centralized and visible.
|
||||
|
||||
**Scope**
|
||||
- `ui/goose2/src/features/chat/stores/chatSessionStore.ts`
|
||||
- `ui/goose2/src/features/chat/hooks/useChat.ts`
|
||||
- `ui/goose2/src/features/chat/hooks/useResolvedAgentModelPicker.ts`
|
||||
- `ui/goose2/src/features/chat/hooks/useChatSessionController.ts`
|
||||
- New `ui/goose2/src/features/chat/stores/chatSessionOperations.ts`
|
||||
|
||||
**Out Of Scope**
|
||||
- Do not split `chatSessionStore` UI state yet.
|
||||
- Do not refactor `projectStore`.
|
||||
- Do not change ACP API behavior.
|
||||
- Do not change title generation, archive, unarchive, or project assignment UX.
|
||||
- Do not solve `chatSessionStore` context-panel or workspace UI ownership here; that belongs in Phase 4.
|
||||
- Do not introduce rollback, refresh-on-failure, or broad user-visible error policy in this phase.
|
||||
- Do not use `commands` naming for the new module, because Tauri backend commands already use that term.
|
||||
|
||||
**Execution Steps**
|
||||
|
||||
1. Identify all `updateSession` call sites.
|
||||
- Classify each call as local-only, title rename, project update, timestamp update, provider/model update, archive-related, or other.
|
||||
- Keep a note of calls that also reveal broad `chatSessionStore` ownership, but do not expand this phase beyond mutation semantics.
|
||||
- Current classification:
|
||||
- manual title update: `AppShell.tsx` should move to `updateSessionTitle`.
|
||||
- project assignment: `AppShell.tsx` and `useChatSessionController.ts` should move to `updateSessionProject`.
|
||||
- backend/session-info title metadata: `acpSessionInfoUpdate.ts` should be local-only `patchSession`.
|
||||
- generated local title, timestamps, provider/model/persona metadata, ACP notification model updates, and test setup should be local-only `patchSession`.
|
||||
- mixed pending patch in `useChatSessionController.ts` should split `projectId` into `updateSessionProject` and keep provider/persona/model fields as `patchSession`.
|
||||
|
||||
2. Add a local-only action.
|
||||
- Introduce `patchSession` in `chatSessionStore`.
|
||||
- It should only update Zustand state.
|
||||
- It should not inspect patch fields for backend meaning.
|
||||
- Keep the existing shallow-merge local update behavior exactly the same for now.
|
||||
|
||||
3. Keep compatibility while migrating.
|
||||
- Temporarily keep `updateSession`.
|
||||
- Have `updateSession` delegate to `patchSession` only after persisted call sites have moved to operations.
|
||||
- Remove `updateSession` when no production or test call sites remain.
|
||||
|
||||
4. Add explicit operation functions.
|
||||
- Add `chatSessionOperations.ts`.
|
||||
- Add `updateSessionTitle(sessionId, title)`.
|
||||
- Call backend rename first.
|
||||
- After success, call `patchSession(sessionId, { title, userSetName: true })`.
|
||||
- Let errors throw so UI call sites can decide how to surface failure.
|
||||
- Add `updateSessionProject(sessionId, projectId)`.
|
||||
- Call backend project update first.
|
||||
- After success, call `patchSession(sessionId, { projectId })`.
|
||||
- Let errors throw so UI call sites can decide how to surface failure.
|
||||
- Import backend API functions with aliases if needed to avoid name conflicts, for example `renameSession as updateSessionTitleApi`.
|
||||
- Leave archive/unarchive in the store initially unless migration remains simple after title/project cleanup.
|
||||
|
||||
5. Migrate call sites one category at a time.
|
||||
- Timestamp-only and UI/local patches should use `patchSession`.
|
||||
- Manual title rename should call `updateSessionTitle`.
|
||||
- Project assignment should call `updateSessionProject`.
|
||||
- Backend/session-info updates should use `patchSession` only, even when the patch includes `title` or `userSetName`.
|
||||
- `userSetName` alone should never imply a backend call.
|
||||
- Archive flows can stay on existing explicit `archiveSession` / `unarchiveSession` until title/project ambiguity is gone.
|
||||
|
||||
6. Remove hidden side effects from `updateSession`.
|
||||
- Once call sites are migrated, remove `updateSession`.
|
||||
- Avoid leaving two names with different implied semantics.
|
||||
|
||||
7. Add focused tests.
|
||||
- `patchSession` does not call backend APIs.
|
||||
- `updateSessionTitle` calls backend rename before patching local state.
|
||||
- `updateSessionTitle` does not patch local state if backend rename rejects.
|
||||
- `updateSessionProject` calls backend project update before patching local state.
|
||||
- `updateSessionProject` does not patch local state if backend project update rejects.
|
||||
- Backend/session-info updates with `title` and/or `userSetName` are local-only.
|
||||
|
||||
8. Record failure-policy follow-up.
|
||||
- After this phase, evaluate whether archive and unarchive should also move to operation functions and whether persisted operations need user-visible error handling.
|
||||
- Track that decision in `phase-5-session-workflow-failure-policy.md`.
|
||||
|
||||
**Validation**
|
||||
- `rg "updateSession\\(" ui/goose2/src`
|
||||
- `cd ui/goose2 && pnpm test -- chatSessionStore useChat useChatSessionController`
|
||||
|
||||
**Success Criteria**
|
||||
- Generic local session patching is local-only.
|
||||
- Backend writes happen through explicitly named operations.
|
||||
- Existing chat rename, project update, archive, and unarchive flows still work.
|
||||
@@ -0,0 +1,101 @@
|
||||
**Phase 4: Split The Clearest Store Boundaries**
|
||||
|
||||
**Status**
|
||||
- Not started.
|
||||
|
||||
**Goal**
|
||||
- Move clearly UI-only state out of domain-heavy stores.
|
||||
- Keep store splits narrow and behavior-preserving.
|
||||
- Use the Phase 2 selector audit to distinguish UI-state extraction from deeper domain-model cleanup.
|
||||
- Revisit component responsibility after UI state moves, especially where Phase 2 identified components that still depend on many store concerns.
|
||||
|
||||
**Scope**
|
||||
- `ui/goose2/src/features/agents/stores/agentStore.ts`
|
||||
- `ui/goose2/src/features/agents/stores/agentSelectors.ts` if Phase 2 adds it
|
||||
- New `ui/goose2/src/features/agents/stores/agentUiStore.ts`
|
||||
- `ui/goose2/src/features/chat/stores/chatSessionStore.ts`
|
||||
- `ui/goose2/src/features/chat/stores/chatSessionSelectors.ts`
|
||||
- `ui/goose2/src/features/chat/stores/chatSelectors.ts` if `chatStore` boundaries are revisited
|
||||
- New `ui/goose2/src/features/chat/stores/chatSessionUiStore.ts`
|
||||
- Consumers of persona editor state, context panel state, and active workspace state.
|
||||
|
||||
**Out Of Scope**
|
||||
- Do not split personas, agents, providers, or selected provider yet.
|
||||
- Do not move `activeSessionId` unless this phase's boundary review proves it is safe.
|
||||
- Do not split `chatStore` immediately.
|
||||
- Do not refactor project orchestration in this phase.
|
||||
- Do not solve the full persona vs agent vs provider domain model in the first store split; record the model problem and keep the initial extraction narrow.
|
||||
|
||||
**Execution Steps**
|
||||
|
||||
1. Split `agentStore` UI state first.
|
||||
- Phase 2 identified `agentStore` as mixing personas, agents, providers, selected provider, active agent, and persona editor UI state.
|
||||
- Treat persona editor state as the safest first extraction, not as the full solution to the domain model.
|
||||
- Move these fields to `agentUiStore`:
|
||||
- `personaEditorOpen`
|
||||
- `editingPersona`
|
||||
- `personaEditorMode`
|
||||
- Move these actions to `agentUiStore`:
|
||||
- `openPersonaEditor`
|
||||
- `closePersonaEditor`
|
||||
- Keep catalog/provider state in `agentStore`.
|
||||
|
||||
2. Update agent UI consumers.
|
||||
- Update `AgentsView.tsx` and related components to read editor state from `agentUiStore`.
|
||||
- Update any controller code that opens or closes the persona editor.
|
||||
- Keep persona CRUD and provider selection on `agentStore`.
|
||||
- Update any Phase 2 selector files so `agentSelectors.ts` contains only state that remains in `agentStore`. If UI selectors were avoided in Phase 2, this should be a small verification step.
|
||||
|
||||
3. Validate the `agentStore` split.
|
||||
- Tests for domain state should not need editor fields.
|
||||
- Add minimal `agentUiStore` tests if behavior is not trivially covered by UI tests.
|
||||
- Recheck `agentStore.isLoading`. Phase 2 found it may be a legacy/general loading flag because personas, agents, and providers already have specific loading fields. Remove or defer it only after confirming consumers and tests do not depend on it.
|
||||
- Recheck derived helper methods such as `getActiveAgent`, `getAgentsByPersona`, `getBuiltinPersonas`, and `getCustomPersonas`. Keep them for imperative `getState()` usage if useful, but React consumers should prefer selectors or pure helpers.
|
||||
|
||||
4. Split `chatSessionStore` UI state second.
|
||||
- Phase 2 identified `chatSessionStore` as mixing session records, hydration/loading, active selection, creation/archive/mutation actions, context-panel state, and workspace UI state.
|
||||
- This phase should extract only the clearly UI-only context panel and workspace state.
|
||||
- Move these fields to `chatSessionUiStore`:
|
||||
- `contextPanelOpenBySession`
|
||||
- `activeWorkspaceBySession`
|
||||
- Move these actions to `chatSessionUiStore`:
|
||||
- `setContextPanelOpen`
|
||||
- `setActiveWorkspace`
|
||||
- `clearActiveWorkspace`
|
||||
- Keep session records and active session selection in `chatSessionStore`.
|
||||
|
||||
5. Update chat UI consumers.
|
||||
- Update `ChatView.tsx`, `ContextPanel.tsx`, `useChatSessionController.ts`, and `useChat.ts` if they read the moved fields/actions.
|
||||
- Keep session creation, loading, archive, and rename behavior unchanged.
|
||||
- Update `chatSessionSelectors.ts` so selectors for moved UI state live with `chatSessionUiStore` or are removed. Keep session record selectors with `chatSessionStore`.
|
||||
- Recheck session-derived values after the split. Active session and Home session should still be derived from `sessions + activeSessionId` or `sessions + homeSessionId`; do not add duplicate stored session objects.
|
||||
- Recheck sidebar session grouping after UI state moves. If grouping remains complex, prefer a pure helper or selector-like utility over adding grouped sidebar rows to store state.
|
||||
|
||||
6. Re-evaluate `chatStore` only after the two safe splits.
|
||||
- Phase 2 identified `chatStore` as mixing messages, runtime, drafts, queue, loading, scroll targets, and cleanup.
|
||||
- Phase 2 also found duplicated active-session selection: both `chatStore` and `chatSessionStore` store `activeSessionId`. Decide which store should own active session selection, or explicitly document why both must remain synchronized.
|
||||
- Do not split message/runtime/draft state in this phase unless the selector data from Phases 1 and 2 shows a very clear boundary.
|
||||
- If any `chatStore` state moves later, update `chatSelectors.ts` in the same change so selector files continue to reflect real store boundaries.
|
||||
- Record any follow-up observations in the progress tracker.
|
||||
|
||||
7. Record remaining domain-model questions.
|
||||
- Clarify whether personas, agents, providers, selected provider, and active agent need separate stores or clearer naming.
|
||||
- Defer any larger domain-model split to a later focused plan after the UI-only state extraction is complete.
|
||||
- Recheck store helper methods that return derived values, including `chatStore.getActiveMessages`, `chatSessionStore.getActiveSession`, and `chatSessionStore.getArchivedSessions`. Prefer pure helpers/selectors for React reads, but keep imperative helpers if they are still useful in callbacks or non-React code.
|
||||
|
||||
8. Recheck component responsibility after the UI-store splits.
|
||||
- `AgentsView` should be simpler after persona editor UI state moves out of `agentStore`; if it still mixes list filtering, editor orchestration, import/export, delete confirmation, and mutation flows too heavily, record a focused component-decomposition follow-up.
|
||||
- `useChatSessionController`, `ChatView`, and context-panel consumers should be rechecked after context panel and workspace UI state move out of `chatSessionStore`.
|
||||
- Recheck chat/session actions that are owned too high in `AppShell` and prop-drilled through multiple surfaces, especially rename from `SidebarChatRow` and `SessionCard`.
|
||||
- Recheck whether rename/archive/move actions need a surface-level action hook or controller after state boundaries are clearer.
|
||||
- Do not decompose components in this phase unless the split requires it for safe wiring; record the follow-up with concrete files and responsibilities.
|
||||
|
||||
**Validation**
|
||||
- `rg "personaEditorOpen|editingPersona|personaEditorMode|openPersonaEditor|closePersonaEditor" ui/goose2/src`
|
||||
- `rg "contextPanelOpenBySession|activeWorkspaceBySession|setContextPanelOpen|setActiveWorkspace|clearActiveWorkspace" ui/goose2/src`
|
||||
- `cd ui/goose2 && pnpm test -- agentStore chatSessionStore useChatSessionController`
|
||||
|
||||
**Success Criteria**
|
||||
- Persona editor UI state is no longer in `agentStore`.
|
||||
- Context panel and active workspace UI state are no longer in `chatSessionStore`.
|
||||
- Domain store behavior remains unchanged.
|
||||
@@ -0,0 +1,85 @@
|
||||
**Phase 5: Decide Chat Session Workflow Failure Policy**
|
||||
|
||||
**Status**
|
||||
- Not started.
|
||||
|
||||
**Goal**
|
||||
- Decide whether chat-session workflow actions should keep optimistic-without-rollback behavior or move to a stronger consistency policy.
|
||||
- Keep this as a separate behavior-change decision after Phase 3 makes title/project workflow actions explicit.
|
||||
- Revisit archive/unarchive after Phase 4 clarifies store boundaries, because those workflows still live in `chatSessionStore` today.
|
||||
|
||||
**Why This Is Separate**
|
||||
- Phase 3 removes hidden side effects without changing behavior.
|
||||
- Failure-policy changes are user-visible and need intentional product/UX decisions.
|
||||
- Once workflows such as `renameSessionAndPersist` and `updateSessionProjectAndPersist` exist, each policy can be changed in one place.
|
||||
|
||||
**Scope**
|
||||
- Chat-session workflow actions created in Phase 3:
|
||||
- rename session
|
||||
- update session project
|
||||
- Chat-session workflow actions to revisit after Phase 4:
|
||||
- archive session
|
||||
- unarchive session
|
||||
- User-visible error handling for those workflows, if policy changes require it.
|
||||
- Pending/error UI for direct user actions such as rename, if needed.
|
||||
- Focused tests for success and failure behavior.
|
||||
|
||||
**Out Of Scope**
|
||||
- Do not refactor `projectStore`; that remains Phase 6.
|
||||
- Do not standardize all persistence; that remains Phase 7.
|
||||
- Do not change unrelated chat send, replay, or model-selection behavior.
|
||||
|
||||
**Policy Options To Evaluate**
|
||||
|
||||
1. Keep optimistic without rollback.
|
||||
- Local state updates immediately.
|
||||
- Backend failure is logged.
|
||||
- Lowest UI friction, weakest consistency.
|
||||
|
||||
2. Backend first.
|
||||
- Backend succeeds before local state changes.
|
||||
- Strong consistency, slower perceived UI.
|
||||
|
||||
3. Optimistic with rollback.
|
||||
- Local state updates immediately.
|
||||
- Backend failure restores previous local state.
|
||||
- Better consistency, more rollback complexity.
|
||||
|
||||
4. Optimistic with refresh on failure.
|
||||
- Local state updates immediately.
|
||||
- Backend failure reloads sessions from backend.
|
||||
- Simpler than targeted rollback, but may be heavier.
|
||||
|
||||
5. Optimistic with user-visible error.
|
||||
- Local state updates immediately.
|
||||
- Backend failure leaves local state but tells the user.
|
||||
- Makes failure visible without solving consistency.
|
||||
|
||||
**Execution Steps**
|
||||
|
||||
1. Review Phase 3 workflow actions and call sites.
|
||||
- Confirm each workflow has one clear implementation point.
|
||||
- Confirm current optimistic-without-rollback behavior is covered by tests.
|
||||
|
||||
2. Choose policy per workflow.
|
||||
- Rename may be able to use backend-first or rollback.
|
||||
- Project assignment may need extra care because it affects sidebar grouping.
|
||||
- Archive/unarchive should be evaluated after Phase 4. Current behavior is optimistic without rollback: local visibility changes immediately, backend failures are logged, and local state is not restored. Moving these workflows out of `chatSessionStore` should happen after store boundaries are clearer.
|
||||
|
||||
3. Implement one workflow policy at a time.
|
||||
- Keep each behavior change small.
|
||||
- Add focused failure tests before moving to the next workflow.
|
||||
|
||||
4. Add user-visible error handling only if the chosen policy needs it.
|
||||
- Use existing toast/error patterns.
|
||||
- Avoid noisy errors for background refresh-style recovery.
|
||||
- Recheck rename UX. Today `SidebarChatRow` and `SessionCard` close inline edit immediately and invoke a void callback; backend-first rename failure can only be surfaced as a toast unless those components gain pending/error state.
|
||||
|
||||
**Validation**
|
||||
- `cd ui/goose2 && pnpm test -- chatSessionStore useChatSessionController Sidebar`
|
||||
- Manual smoke: rename chat, move chat to project, archive/unarchive chat, force or mock backend failure where feasible.
|
||||
|
||||
**Success Criteria**
|
||||
- Each chat-session workflow has an explicit failure policy.
|
||||
- Failure behavior is tested.
|
||||
- Local/backend consistency behavior is intentional rather than inherited from the old generic `updateSession`.
|
||||
@@ -0,0 +1,82 @@
|
||||
**Phase 6: Refactor Project Store Into Clearer Layers**
|
||||
|
||||
**Status**
|
||||
- Not started.
|
||||
|
||||
**Goal**
|
||||
- Reduce `projectStore` responsibility overload.
|
||||
- Make API orchestration, local state transitions, cache behavior, and mutation policy easier to reason about.
|
||||
- Address Phase 2 findings that project reads are common but the bigger issue is project workflow ownership, not selector syntax.
|
||||
- Revisit project-heavy component responsibility after project-store ownership is clearer.
|
||||
|
||||
**Scope**
|
||||
- `ui/goose2/src/features/projects/stores/projectStore.ts`
|
||||
- New project command or hook module, such as `useProjectCommands.ts` or `projectCommands.ts`
|
||||
- Project consumers such as `ProjectsView.tsx`, `Sidebar.tsx`, and `SettingsModal.tsx`
|
||||
|
||||
**Out Of Scope**
|
||||
- Do not standardize all persistence yet.
|
||||
- Do not change project API contracts.
|
||||
- Do not redesign project UI.
|
||||
- Do not change reorder UX unless the current behavior is clearly buggy.
|
||||
|
||||
**Execution Steps**
|
||||
|
||||
1. Document the current mutation policy.
|
||||
- Include the fact that `AppShell`, `Sidebar`, and project views depend on project list lookup by id, while `projectStore` also owns API orchestration and cache persistence.
|
||||
- `fetchProjects`: pessimistic load with cached bootstrap.
|
||||
- `addProject`, `editProject`, `removeProject`: backend first, local update after success.
|
||||
- `reorderProjects`: local optimistic reorder, persist cache, fire backend reorder without rollback.
|
||||
|
||||
2. Add local state actions.
|
||||
- Introduce local-only store actions with explicit names, for example:
|
||||
- `setProjectsLocal`
|
||||
- `upsertProjectLocal`
|
||||
- `removeProjectLocal`
|
||||
- `reorderProjectsLocal`
|
||||
- `setProjectsLoading`
|
||||
- `setActiveProject`
|
||||
- Keep old public actions temporarily if migration needs to be incremental.
|
||||
|
||||
3. Add project commands.
|
||||
- Move backend workflows into a command-style module or hook.
|
||||
- Commands can call project API modules and then local store actions.
|
||||
- Keep UI call sites simple.
|
||||
|
||||
4. Migrate consumers gradually.
|
||||
- Move `fetchProjects`, `addProject`, `editProject`, and `removeProject` consumers to commands.
|
||||
- Migrate reorder last because it has the least explicit failure behavior today.
|
||||
|
||||
5. Recheck project-related component responsibility.
|
||||
- `AppShell` currently orchestrates project fetch, project creation/edit/archive callbacks, project selection for new chats, sidebar project actions, and chat/session setup. After project-store policy is clarified, decide whether some project orchestration should move into a project controller hook or smaller app-shell child component.
|
||||
- `Sidebar` currently combines project display, session grouping by project, expanded-project persistence, runtime badges, and search resolver support. After project selectors/helpers are settled, decide whether project/session grouping belongs in a pure helper or a smaller sidebar controller.
|
||||
- Recheck project lookup derived values. Project lookup by id should remain derived from `projects + projectId`; if repeated lookup logic is noisy, use a pure helper over the project list instead of storing duplicate active/current project objects.
|
||||
- Recheck `projectStore.getActiveProject`. It is an imperative derived helper over `projects + activeProjectId`; keep it only if useful for `getState()`/callback code, and prefer selectors or pure helpers for React reads.
|
||||
- Record concrete follow-ups instead of doing a broad component refactor inside this phase unless the project-store change requires it.
|
||||
|
||||
6. Make reorder policy explicit.
|
||||
- Preserve current optimistic behavior unless product expectations require a change.
|
||||
- Add a clear comment or function name indicating whether reorder is optimistic without rollback, optimistic with refresh, or pessimistic.
|
||||
- Prefer adding failure handling in a separate small change if needed.
|
||||
|
||||
7. Reduce persistence coupling, but do not standardize persistence yet.
|
||||
- Keep cache behavior working.
|
||||
- Isolate cache reads/writes behind small helper functions if they remain in this phase.
|
||||
|
||||
8. Add focused tests.
|
||||
- Project cache bootstrap.
|
||||
- Add/edit/remove success behavior.
|
||||
- Reorder local ordering and order fields.
|
||||
- Reorder backend call shape.
|
||||
- Failure behavior documented by tests where feasible.
|
||||
|
||||
**Validation**
|
||||
- `cd ui/goose2 && pnpm test -- project`
|
||||
- `rg "useProjectStore\\(\\)" ui/goose2/src`
|
||||
- Manual smoke: create, edit, reorder, and delete a project.
|
||||
|
||||
**Success Criteria**
|
||||
- `projectStore` no longer owns every project workflow directly.
|
||||
- Mutation policy is explicit.
|
||||
- Cache behavior still works.
|
||||
- Project UI behavior is unchanged.
|
||||
@@ -0,0 +1,61 @@
|
||||
**Phase 7: Standardize Persistence Boundaries**
|
||||
|
||||
**Status**
|
||||
- Not started.
|
||||
|
||||
**Goal**
|
||||
- Make durable state intentional and consistent.
|
||||
- Reduce ad hoc localStorage handling where Zustand `persist` or isolated persistence helpers provide a cleaner boundary.
|
||||
|
||||
**Scope**
|
||||
- `ui/goose2/src/features/projects/stores/projectStore.ts`
|
||||
- `ui/goose2/src/features/agents/stores/agentStore.ts`
|
||||
- `ui/goose2/src/features/chat/stores/draftPersistence.ts`
|
||||
- Any persistence introduced or clarified in earlier phases.
|
||||
|
||||
**Out Of Scope**
|
||||
- Do not persist transient runtime state.
|
||||
- Do not persist UI-only modal/open state by default.
|
||||
- Do not convert isolated, well-tested persistence helpers just for consistency.
|
||||
- Do not change product decisions about what survives reload without recording the decision.
|
||||
|
||||
**Execution Steps**
|
||||
|
||||
1. Inventory durable state.
|
||||
- Selected provider.
|
||||
- Chat drafts.
|
||||
- Project cache.
|
||||
- Home session id if relevant to this area.
|
||||
- Model preferences if relevant to this area.
|
||||
|
||||
2. Classify each durable value.
|
||||
- Product preference.
|
||||
- Cache for faster bootstrap.
|
||||
- Draft/user input recovery.
|
||||
- Runtime state that should not be persisted.
|
||||
- UI state that should not be persisted.
|
||||
|
||||
3. Decide persistence mechanism per value.
|
||||
- Use Zustand `persist` only when the persisted data is clearly store-owned.
|
||||
- Keep separate helper modules for persistence that is intentionally outside a store or already isolated.
|
||||
- Use `partialize` and versioning when adopting `persist`.
|
||||
|
||||
4. Migrate one persistence area at a time.
|
||||
- Selected provider is the smallest candidate.
|
||||
- Project cache should wait until project store ownership is clear.
|
||||
- Draft persistence can remain as-is if the helper is clearer than `persist`.
|
||||
|
||||
5. Add tests around migration behavior.
|
||||
- Hydration/default behavior.
|
||||
- Invalid stored data fallback.
|
||||
- Version/migration behavior if `persist` is used.
|
||||
- Non-persistence of transient state.
|
||||
|
||||
**Validation**
|
||||
- `rg "localStorage|getItem|setItem|persist\\(" ui/goose2/src/features`
|
||||
- `cd ui/goose2 && pnpm test -- chatStore agentStore project`
|
||||
|
||||
**Success Criteria**
|
||||
- Durable state choices are documented and minimal.
|
||||
- Persistence code has clear ownership.
|
||||
- No runtime or temporary UI state is accidentally persisted.
|
||||
@@ -0,0 +1,66 @@
|
||||
**Phase 8: Standardize Test Reset Patterns And Close Coverage Gaps**
|
||||
|
||||
**Status**
|
||||
- Not started.
|
||||
|
||||
**Goal**
|
||||
- Make store and hook tests safer after the state boundaries have stabilized.
|
||||
- Replace partial shallow-merge resets with known full-state resets.
|
||||
- Add coverage for risky persistence and mutation policy paths.
|
||||
|
||||
**Scope**
|
||||
- Store tests for chat, chat sessions, agents, projects, providers, and any UI stores introduced in Phase 4.
|
||||
- Hook tests that directly manipulate Zustand stores.
|
||||
- Coverage gaps discovered in Phases 3 through 6.
|
||||
|
||||
**Out Of Scope**
|
||||
- Do not rewrite all tests for style.
|
||||
- Do not add broad snapshot-style coverage.
|
||||
- Do not introduce a large test utility framework unless duplication is clearly painful.
|
||||
|
||||
**Execution Steps**
|
||||
|
||||
1. Add full initial-state reset helpers.
|
||||
- Prefer one helper per store.
|
||||
- Keep helpers easy to update when store shape changes.
|
||||
- Include actions only if the store's reset approach requires them.
|
||||
|
||||
2. Replace partial resets in existing tests.
|
||||
- `agentStore.test.ts`
|
||||
- `usePersonas.test.ts`
|
||||
- `chatStore.test.ts`
|
||||
- Chat hook tests that call `setState` directly.
|
||||
- New store tests from earlier phases.
|
||||
|
||||
3. Add project store coverage.
|
||||
- Cache hydration.
|
||||
- Add/edit/remove local state results.
|
||||
- Reorder semantics.
|
||||
- Reorder failure policy if implemented or documented.
|
||||
|
||||
4. Add provider inventory store coverage if still missing.
|
||||
- `setEntries` replaces inventory.
|
||||
- `mergeEntries` preserves existing providers and updates overlapping providers.
|
||||
- Loading behavior.
|
||||
|
||||
5. Add session mutation orchestration coverage.
|
||||
- Local patch does not call backend APIs.
|
||||
- Rename operation calls backend rename.
|
||||
- Project update operation calls backend project update.
|
||||
- Archive/unarchive operation behavior.
|
||||
|
||||
6. Remove stale assumptions from tests.
|
||||
- Tests should not rely on fields that moved to UI stores.
|
||||
- Tests should reset every store touched by the scenario.
|
||||
- Component-test mocks for Zustand bound hooks should support selector-style calls. Phase 1 moved production code from `useStore()` to `useStore((state) => ...)`, so mocks should accept an optional selector and invoke it with the mocked state shape.
|
||||
- Tests that import Phase 2 selector files should still match the final store boundaries after Phases 4 and 5. Remove or move selector imports if the selected state moved to a new store.
|
||||
- Recheck test reset assumptions around legacy/general fields such as `agentStore.isLoading`. If Phase 4 removes or narrows those fields, reset helpers should not preserve stale state shape.
|
||||
|
||||
**Validation**
|
||||
- `cd ui/goose2 && pnpm test`
|
||||
- `rg "setState\\(\\{" ui/goose2/src/features ui/goose2/src/shared`
|
||||
|
||||
**Success Criteria**
|
||||
- Store tests reset from known initial state.
|
||||
- Risky mutation and persistence paths have direct coverage.
|
||||
- Test setup matches the final store boundaries.
|
||||
@@ -0,0 +1,51 @@
|
||||
**Phase 9: Optional Immer Adoption**
|
||||
|
||||
**Status**
|
||||
- Not started.
|
||||
|
||||
**Goal**
|
||||
- Improve nested update readability only where structural cleanup did not solve the pain.
|
||||
- Avoid using Immer as a substitute for better store boundaries.
|
||||
|
||||
**Scope**
|
||||
- `ui/goose2/src/features/chat/stores/chatStore.ts`
|
||||
- Possibly `ui/goose2/src/features/chat/stores/chatSessionStore.ts`
|
||||
|
||||
**Out Of Scope**
|
||||
- Do not add Immer before earlier phases are complete.
|
||||
- Do not use Immer to justify keeping a broad store.
|
||||
- Do not convert flat stores where object spread is already clear.
|
||||
- Do not make dependency changes manually in `Cargo.toml`; for frontend packages follow the repo's package workflow.
|
||||
|
||||
**Execution Steps**
|
||||
|
||||
1. Reassess update complexity after Phases 1 through 7.
|
||||
- Check whether `chatStore` nested updates are still hard to read.
|
||||
- Check whether store splits or selectors already reduced the pain enough.
|
||||
|
||||
2. Decide whether Immer is worth the dependency and pattern change.
|
||||
- Adopt only if it materially improves readability.
|
||||
- Skip this phase if the remaining updates are acceptable.
|
||||
|
||||
3. If adopting, start with one store.
|
||||
- Prefer `chatStore` first.
|
||||
- Convert a small group of related nested updates.
|
||||
- Keep behavior exactly the same.
|
||||
|
||||
4. Add or update focused tests.
|
||||
- Messages update correctly.
|
||||
- Runtime state updates correctly.
|
||||
- Drafts and cleanup still work.
|
||||
- No accidental mutation leaks between sessions.
|
||||
|
||||
5. Reassess before expanding.
|
||||
- Only convert `chatSessionStore` if there is still meaningful nested update boilerplate.
|
||||
|
||||
**Validation**
|
||||
- `cd ui/goose2 && pnpm test -- chatStore chatSessionStore useChat`
|
||||
- Focused code review for accidental mutation or changed reference behavior.
|
||||
|
||||
**Success Criteria**
|
||||
- Update logic is easier to read.
|
||||
- Behavior is unchanged.
|
||||
- Immer is used selectively and not as an architectural workaround.
|
||||
@@ -0,0 +1,154 @@
|
||||
**Goose2 Zustand Refactor Progress**
|
||||
|
||||
**Current Status**
|
||||
- Phase 1 implementation complete.
|
||||
- Phase 2 implementation complete.
|
||||
- Phase 3 implementation complete.
|
||||
|
||||
**Documents**
|
||||
- Added master review: `goose2-zustand-state-management-review.md`
|
||||
- Added master plan: `goose2-zustand-state-management-improvement-plan.md`
|
||||
- Added per-phase execution plans:
|
||||
- `phase-1-selector-cleanup.md`
|
||||
- `phase-2-selector-read-layer.md`
|
||||
- `phase-3-session-side-effects.md`
|
||||
- `phase-4-store-boundaries.md`
|
||||
- `phase-5-session-workflow-failure-policy.md`
|
||||
- `phase-6-project-store.md`
|
||||
- `phase-7-persistence.md`
|
||||
- `phase-8-test-reset-coverage.md`
|
||||
- `phase-9-optional-immer.md`
|
||||
|
||||
**Decisions**
|
||||
- Keep simple numbered phases without `A`/`B` sub-phases.
|
||||
- Do not make test reset cleanup Phase 1.
|
||||
- Clean up test reset patterns opportunistically when a refactor PR already touches those tests.
|
||||
- Keep the dedicated test reset and coverage pass as Phase 8.
|
||||
- Create per-phase execution files so each phase has scope, guardrails, validation, and success criteria.
|
||||
- In Phase 1, keep existing hook/component public contracts unchanged. For example, `usePersonas()` should keep returning `personas` and `isLoading` even though its current production consumer only uses command functions.
|
||||
- For Phase 3, use explicit `chatSessionOperations.ts` functions for backend-aware chat-session workflows. Use `patchSession` for local-only store updates. For title and project updates, call backend first and patch local state only after success.
|
||||
|
||||
**Phase Status**
|
||||
|
||||
| Phase | Name | Status | Notes |
|
||||
| --- | --- | --- | --- |
|
||||
| 1 | Remove whole-store subscriptions | Complete | Replaced broad subscriptions in `usePersonas.ts`, `useChat.ts`, `Sidebar.tsx`, and `AppShell.tsx`. |
|
||||
| 2 | Introduce selector-first read layer | Complete | Added initial `chatSessionStore`, `chatStore`, `agentStore`, and `projectStore` selector helpers. Deferred derived values, store-boundary issues, and component-boundary issues to later phases. |
|
||||
| 3 | Separate session side effects | Complete | Removed generic `updateSession`; local patches use `patchSession`, and title/project persistence uses explicit operations. |
|
||||
| 4 | Split clearest store boundaries | Pending | Split `agentStore` UI state, then `chatSessionStore` UI state. |
|
||||
| 5 | Decide session workflow failure policy | Pending | Behavior-policy follow-up after Phase 4 clarifies session-store boundaries. |
|
||||
| 6 | Refactor project store | Pending | Make mutation policy explicit. |
|
||||
| 7 | Standardize persistence | Pending | Do after boundaries are clearer. |
|
||||
| 8 | Test reset and coverage | Pending | Dedicated cleanup after boundary changes stabilize. |
|
||||
| 9 | Optional Immer | Pending | Skip unless update readability still justifies it. |
|
||||
|
||||
**Known Current-Code Findings To Recheck Before Phase 1**
|
||||
- Broad subscription scan is clean after Phase 1:
|
||||
- `rg "use[A-Za-z0-9]+Store\\(\\)" ui/goose2/src`
|
||||
- `useShallow` is currently unused in `ui/goose2/src`.
|
||||
- Phase 3 removed `chatSessionStore.updateSession`; local-only session changes now use `patchSession`.
|
||||
- `projectStore` currently owns local cache, API orchestration, state, active selection, and reorder behavior.
|
||||
- `agentStore` currently mixes catalog/provider state with persona editor UI state.
|
||||
|
||||
**Next Step**
|
||||
- Start Phase 4 using `phase-4-store-boundaries.md`.
|
||||
|
||||
**Validation Log**
|
||||
- Phase 1 broad subscription scan: clean.
|
||||
- Phase 1 TypeScript check: `cd ui/goose2 && pnpm exec tsc --noEmit` passed.
|
||||
- Phase 1 focused tests: `cd ui/goose2 && pnpm test -- useChat usePersonas Sidebar` passed.
|
||||
- Phase 2 Step 1 TypeScript check: `cd ui/goose2 && pnpm exec tsc --noEmit` passed.
|
||||
- Phase 2 Step 2 TypeScript check: `cd ui/goose2 && pnpm exec tsc --noEmit` passed.
|
||||
- Phase 2 Step 3A TypeScript check: `cd ui/goose2 && pnpm exec tsc --noEmit` passed.
|
||||
- Phase 2 Step 3B TypeScript check: `cd ui/goose2 && pnpm exec tsc --noEmit` passed.
|
||||
- Phase 2 Step 4A TypeScript check: `cd ui/goose2 && pnpm exec tsc --noEmit` passed.
|
||||
- Phase 2 Step 4B TypeScript check: `cd ui/goose2 && pnpm exec tsc --noEmit` passed.
|
||||
- Phase 2 focused tests: `cd ui/goose2 && pnpm test -- useChat usePersonas Sidebar useProviderInventory` passed.
|
||||
- Phase 2 `useProviderInventory` / `useShallow` assessment complete: no code change recommended.
|
||||
- Phase 2 final broad subscription scan: `rg "use[A-Za-z0-9]+Store\\(\\)" ui/goose2/src` found no matches.
|
||||
- Phase 2 final `useShallow` scan: `rg "useShallow|zustand/shallow" ui/goose2/src` found no matches, as expected.
|
||||
- Phase 3 TypeScript check: `cd ui/goose2 && pnpm exec tsc --noEmit` passed.
|
||||
- Phase 3 focused tests: `cd ui/goose2 && pnpm test -- chatSessionOperations chatSessionStore` passed; Vitest ran 109 files / 632 tests.
|
||||
- Phase 3 final generic `updateSession` scan: no `chatSessionStore.updateSession` action or call sites remain; only explicit `updateSessionTitle` / `updateSessionProject` operation/API names remain.
|
||||
|
||||
**Follow-Ups To Revisit Later**
|
||||
- Revisit whether `usePersonas()` should remain both a read hook and command hook. Today `AgentsView` already reads `personas` and `personasLoading` through direct store selectors and only uses `usePersonas()` for `createPersona`, `updatePersona`, `deletePersona`, and `refreshFromDisk`. Do not change that contract during Phase 1.
|
||||
- Revisit the sidebar runtime selector in Phase 2. Phase 1 selects the whole `sessionStateById` record to avoid a complex derived selector, which is still narrower than the whole `chatStore`; Phase 2 should evaluate whether visible sidebar session items need a dedicated selector/helper.
|
||||
- Keep selector fallbacks stable. Phase 1 found that returning a fresh `[]` from a selector fallback in `useChat` can trigger React snapshot churn; use module-level constants or derive fallback values outside the selector.
|
||||
- In Phase 8, keep component-test store mocks selector-aware. Phase 1 production code now calls bound hooks with selectors, so mocks need to support `useStore((state) => ...)` as well as any remaining direct mock patterns.
|
||||
- Phase 1 removed dead code exposed by the selector cleanup: `retryLastMessage`, `buildSkillRetryOptions`, and the unused `findLastIndex` helper.
|
||||
- After Phase 3 creates explicit title/project workflow actions, revisit backend failure policy for rename and project assignment. Options include backend-first, optimistic with rollback, refresh-on-failure, or optimistic with user-visible error.
|
||||
- After Phase 4 clarifies `chatSessionStore` boundaries, revisit archive/unarchive. They currently remain store actions with API calls and optimistic-without-rollback behavior: local visibility changes immediately, backend failures are logged, and local state is not restored.
|
||||
|
||||
**Phase 2 Audit Notes**
|
||||
- Completed Step 1:
|
||||
- Added `chatSessionSelectors.ts` with `selectSessions`, `selectActiveSessionId`, `selectHasHydratedSessions`, and `selectSessionsLoading`.
|
||||
- Updated `AppShell.tsx` and `Sidebar.tsx` to use those simple field selectors.
|
||||
- Completed Step 2:
|
||||
- Added `chatSelectors.ts` with `selectMessagesBySession` and `selectSessionStateById`.
|
||||
- Updated `AppShell.tsx` and `Sidebar.tsx` to use those simple field selectors.
|
||||
- Completed Step 3A:
|
||||
- Added `agentSelectors.ts` with `selectPersonas` and `selectPersonasLoading`.
|
||||
- Updated `usePersonas.ts`, `AgentsView.tsx`, and `useChatSessionController.ts` to use persona domain selectors.
|
||||
- Left persona editor UI state selectors inline so Phase 4 extraction remains visible.
|
||||
- Completed Step 3B:
|
||||
- Added `selectSelectedProvider` to `agentSelectors.ts`.
|
||||
- Updated `AppShell.tsx` and `useProviderSelection.ts` to use the selected-provider selector.
|
||||
- Left provider lists/loading/actions inline pending later `agentStore` boundary work.
|
||||
- Completed Step 4A:
|
||||
- Renamed the internal `ProjectState` type to exported `ProjectStore` because the type includes state and actions.
|
||||
- Added `projectSelectors.ts` with `selectProjects`.
|
||||
- Updated direct `projects` field reads in `AppShell.tsx`, `Sidebar.tsx`, `SkillsView.tsx`, `SessionHistoryView.tsx`, and `useChatSessionController.ts`.
|
||||
- Left project actions, loading, and derived project lookup reads inline pending Phase 6 project-store ownership work.
|
||||
- Completed Step 4B:
|
||||
- Updated `SessionHistoryView.tsx` to use existing `selectSessions` and `selectMessagesBySession`.
|
||||
- A scan found no remaining exact inline reads for selector helpers already created.
|
||||
- Left derived/id-dependent reads, UI-state reads, one-off loading reads, and action selectors inline.
|
||||
- Completed `useProviderInventory` / `useShallow` assessment:
|
||||
- `useProviderInventory` reads `entries` and `loading` with separate narrow selectors.
|
||||
- Grouping these into an object selector with `useShallow` would not meaningfully reduce rerenders because consumers should still update when either value changes.
|
||||
- Kept the hook unchanged.
|
||||
- Simple selector candidates:
|
||||
- chat: `selectMessagesBySession`, `selectSessionStateById`
|
||||
- chat sessions: `selectSessions`, `selectActiveSessionId`, `selectHasHydratedSessions`, `selectSessionsLoading`
|
||||
- agents: `selectPersonas`, `selectPersonasLoading`, `selectSelectedProvider`
|
||||
- projects: `selectProjects`, `selectFetchProjects`, `selectReorderProjects`
|
||||
- Derived values should stay derived, not become stored attributes:
|
||||
- active session from `sessions + activeSessionId`
|
||||
- Home session from `sessions + homeSessionId`
|
||||
- project lookup by id from `projects + projectId`
|
||||
- sidebar grouped session rows from `sessions + messagesBySession + sessionStateById + project ids`
|
||||
- Phase 4 should recheck active/Home session derivation and sidebar grouping after chat/session UI state moves.
|
||||
- Phase 6 should recheck project lookup helpers after project-store ownership is clearer.
|
||||
- Duplicated or legacy state to revisit later:
|
||||
- `chatStore.activeSessionId` and `chatSessionStore.activeSessionId` duplicate active-session selection. Track for Phase 4 boundary review.
|
||||
- `agentStore.isLoading` may be a legacy/general loading flag now that personas, agents, and providers have specific loading fields. Track for Phase 4 and test-reset cleanup in Phase 8.
|
||||
- Imperative derived helper methods to revisit later:
|
||||
- `chatStore.getActiveMessages`
|
||||
- `chatSessionStore.getActiveSession`
|
||||
- `chatSessionStore.getArchivedSessions`
|
||||
- `projectStore.getActiveProject`
|
||||
- `agentStore.getActiveAgent`
|
||||
- `agentStore.getAgentsByPersona`
|
||||
- `agentStore.getBuiltinPersonas`
|
||||
- `agentStore.getCustomPersonas`
|
||||
- Keep these only where useful for `getState()`/callback code; prefer selectors or pure helpers for React reads.
|
||||
- Store-boundary signals to carry into later phases:
|
||||
- `agentStore` mixes personas, providers, active agent, selected provider, and persona editor UI. Track for Phase 4.
|
||||
- `chatStore` mixes messages, runtime, drafts, queue, loading, scroll targets, and cleanup. Track for Phase 4 / Phase 9 reassessment.
|
||||
- `chatSessionStore` mixes session records, hydration/loading, active selection, creation, archive, mutation, context panel state, and workspace UI. Track for Phase 3 and Phase 4.
|
||||
- Phase 2 guardrail:
|
||||
- Selectors should not hide real feature complexity. For `agentStore`, use selectors only for stable repeated domain reads such as personas and selected provider.
|
||||
- Do not add selectors for persona editor UI state in Phase 2; keep that visible for Phase 4 UI-store extraction.
|
||||
- If a feature still needs many store dependencies after selector cleanup, track it as a component/store boundary issue instead of adding more selectors.
|
||||
- Selector files created in Phase 2 are part of the read layer, not proof that the current store boundaries are final.
|
||||
- When Phase 4 or Phase 6 moves state across stores, update, move, or delete the corresponding Phase 2 selectors in the same change.
|
||||
- Component-boundary signals to carry into later phases:
|
||||
- `AppShell` is orchestration-heavy: projects, sessions, Home setup, provider/model choice, keyboard shortcuts, dialogs, sidebar wiring, and chat activation. Track as a later component decomposition concern.
|
||||
- `Sidebar` performs visible-session filtering, project grouping, runtime badge derivation, search resolver creation, and expanded-project persistence. Track derived helpers during Phase 2 and possible component decomposition later.
|
||||
- Rename behavior is owned high in `AppShell` and prop-drilled through `Sidebar`/sidebar sections to `SidebarChatRow`, and through `SessionHistoryView` to `SessionCard`. Track for later component/action-controller cleanup.
|
||||
- Rename row components close edit mode immediately and expose void callbacks, so backend-first rename failures cannot currently show inline pending/error state. Track UX policy in Phase 5.
|
||||
- Phase 2 implementation plan:
|
||||
- Start with small chat-session selector helpers because they are simple field selectors.
|
||||
- Refactor only `AppShell.tsx` and `Sidebar.tsx` first.
|
||||
- Do not use selectors to hide the larger Phase 3-5 architecture problems.
|
||||
Reference in New Issue
Block a user