From 38423edd5563dcd4aecfec592b2c4e15987a0ec0 Mon Sep 17 00:00:00 2001 From: morgmart <98432065+morgmart@users.noreply.github.com> Date: Wed, 1 Apr 2026 11:37:03 -0700 Subject: [PATCH] centralize provider selection state in agentStore (#54) * feat: centralize provider selection state in agentStore Move provider discovery and selection from per-component state to a shared store, ensuring the selected provider persists across screens and is loaded once on app startup. Adds loading state for the provider dropdown and removes hardcoded provider labels in favor of API data. * fix: restore default provider after persona override --- ui/goose2/src/app/AppShell.tsx | 26 +------- ui/goose2/src/app/hooks/useAppStartup.ts | 38 +++++++++++ .../agents/hooks/useProviderSelection.ts | 31 +++++++++ .../src/features/agents/stores/agentStore.ts | 65 +++++++++++++++++++ .../src/features/chat/stores/chatStore.ts | 9 ++- ui/goose2/src/features/chat/ui/ChatInput.tsx | 3 + .../features/chat/ui/ChatInputSelector.tsx | 3 + .../src/features/chat/ui/ChatInputToolbar.tsx | 19 ++---- ui/goose2/src/features/chat/ui/ChatView.tsx | 45 +++++-------- .../src/features/home/ui/HomeScreen.test.tsx | 63 ++++++++++++++++++ ui/goose2/src/features/home/ui/HomeScreen.tsx | 50 +++++++------- 11 files changed, 258 insertions(+), 94 deletions(-) create mode 100644 ui/goose2/src/app/hooks/useAppStartup.ts create mode 100644 ui/goose2/src/features/agents/hooks/useProviderSelection.ts diff --git a/ui/goose2/src/app/AppShell.tsx b/ui/goose2/src/app/AppShell.tsx index ee9d58328c..afc437c0b3 100644 --- a/ui/goose2/src/app/AppShell.tsx +++ b/ui/goose2/src/app/AppShell.tsx @@ -18,6 +18,7 @@ import { useProjectStore } from "@/features/projects/stores/projectStore"; import { findReusableNewChatSession } from "@/features/chat/lib/newChat"; import { getSessionMessages } from "@/shared/api/chat"; import type { Tab } from "@/features/tabs/types"; +import { useAppStartup } from "./hooks/useAppStartup"; export type AppView = "home" | "chat" | "skills" | "agents" | "projects"; @@ -75,30 +76,7 @@ export function AppShell({ children }: { children?: React.ReactNode }) { } }, []); - // Load sessions, personas, and tab state on mount - useEffect(() => { - (async () => { - // Load personas eagerly so they're available in HomeScreen and ChatView - const personaStore = useAgentStore.getState(); - personaStore.setPersonasLoading(true); - try { - const { listPersonas } = await import("@/shared/api/agents"); - const personas = await listPersonas(); - personaStore.setPersonas(personas); - } catch (err) { - console.error("Failed to load personas on startup:", err); - } finally { - personaStore.setPersonasLoading(false); - } - - const { loadSessions, loadTabState } = useChatSessionStore.getState(); - await loadSessions(); - await loadTabState(); - // Always start on the Home screen — tabs are restored in the tab bar - // but no tab is selected until the user clicks one. - useChatSessionStore.getState().setActiveTab(null); - })(); - }, []); + useAppStartup(); useEffect(() => { projectStore.fetchProjects(); diff --git a/ui/goose2/src/app/hooks/useAppStartup.ts b/ui/goose2/src/app/hooks/useAppStartup.ts new file mode 100644 index 0000000000..ed41f84e1c --- /dev/null +++ b/ui/goose2/src/app/hooks/useAppStartup.ts @@ -0,0 +1,38 @@ +import { useEffect } from "react"; +import { useAgentStore } from "@/features/agents/stores/agentStore"; +import { useChatSessionStore } from "@/features/chat/stores/chatSessionStore"; + +export function useAppStartup() { + useEffect(() => { + (async () => { + const store = useAgentStore.getState(); + + store.setPersonasLoading(true); + try { + const { listPersonas } = await import("@/shared/api/agents"); + const personas = await listPersonas(); + store.setPersonas(personas); + } catch (err) { + console.error("Failed to load personas on startup:", err); + } finally { + store.setPersonasLoading(false); + } + + store.setProvidersLoading(true); + try { + const { discoverAcpProviders } = await import("@/shared/api/acp"); + const providers = await discoverAcpProviders(); + store.setProviders(providers); + } catch (err) { + console.error("Failed to load ACP providers on startup:", err); + } finally { + store.setProvidersLoading(false); + } + + const { loadSessions, loadTabState } = useChatSessionStore.getState(); + await loadSessions(); + await loadTabState(); + useChatSessionStore.getState().setActiveTab(null); + })(); + }, []); +} diff --git a/ui/goose2/src/features/agents/hooks/useProviderSelection.ts b/ui/goose2/src/features/agents/hooks/useProviderSelection.ts new file mode 100644 index 0000000000..060547eafd --- /dev/null +++ b/ui/goose2/src/features/agents/hooks/useProviderSelection.ts @@ -0,0 +1,31 @@ +import { useCallback } from "react"; +import { useAgentStore } from "../stores/agentStore"; + +export function useProviderSelection() { + const providers = useAgentStore((s) => s.providers); + const providersLoading = useAgentStore((s) => s.providersLoading); + const selectedProvider = useAgentStore((s) => s.selectedProvider); + const storeSetSelectedProvider = useAgentStore((s) => s.setSelectedProvider); + + const setSelectedProvider = useCallback( + (providerId: string) => { + storeSetSelectedProvider(providerId, true); + }, + [storeSetSelectedProvider], + ); + + const setSelectedProviderWithoutPersist = useCallback( + (providerId: string) => { + storeSetSelectedProvider(providerId, false); + }, + [storeSetSelectedProvider], + ); + + return { + providers, + providersLoading, + selectedProvider, + setSelectedProvider, + setSelectedProviderWithoutPersist, + }; +} diff --git a/ui/goose2/src/features/agents/stores/agentStore.ts b/ui/goose2/src/features/agents/stores/agentStore.ts index 138893339d..e0f535529c 100644 --- a/ui/goose2/src/features/agents/stores/agentStore.ts +++ b/ui/goose2/src/features/agents/stores/agentStore.ts @@ -1,5 +1,35 @@ import { create } from "zustand"; import type { Persona, Agent } from "@/shared/types/agents"; +import type { AcpProvider } from "@/shared/api/acp"; + +const PROVIDER_STORAGE_KEY = "goose:defaultProvider"; +const FALLBACK_PROVIDER = "goose"; + +export function getStoredProvider(providers: AcpProvider[] = []): string { + try { + const storedProvider = + localStorage.getItem(PROVIDER_STORAGE_KEY) ?? FALLBACK_PROVIDER; + + if ( + providers.length === 0 || + providers.some((provider) => provider.id === storedProvider) + ) { + return storedProvider; + } + + return providers[0]?.id ?? FALLBACK_PROVIDER; + } catch { + return providers[0]?.id ?? FALLBACK_PROVIDER; + } +} + +function persistProvider(providerId: string): void { + try { + localStorage.setItem(PROVIDER_STORAGE_KEY, providerId); + } catch { + // localStorage may be unavailable + } +} interface AgentStoreState { // Personas @@ -10,6 +40,13 @@ interface AgentStoreState { agents: Agent[]; agentsLoading: boolean; + // ACP Providers (cached) + providers: AcpProvider[]; + providersLoading: boolean; + + // Selected provider (shared across all chat screens) + selectedProvider: string; + // Active agent for current chat activeAgentId: string | null; @@ -36,6 +73,11 @@ interface AgentStoreActions { removeAgent: (id: string) => void; setAgentsLoading: (loading: boolean) => void; + // Provider management + setProviders: (providers: AcpProvider[]) => void; + setProvidersLoading: (loading: boolean) => void; + setSelectedProvider: (providerId: string, persist?: boolean) => void; + // Active agent setActiveAgent: (id: string | null) => void; getActiveAgent: () => Agent | null; @@ -63,6 +105,9 @@ export const useAgentStore = create((set, get) => ({ personasLoading: false, agents: [], agentsLoading: false, + providers: [], + providersLoading: false, + selectedProvider: getStoredProvider(), activeAgentId: null, isLoading: false, personaEditorOpen: false, @@ -112,6 +157,26 @@ export const useAgentStore = create((set, get) => ({ setAgentsLoading: (agentsLoading) => set({ agentsLoading }), + // Provider management + setProviders: (providers) => { + const { selectedProvider } = get(); + const isValid = providers.some((p) => p.id === selectedProvider); + if (!isValid && providers.length > 0) { + const fallback = providers[0].id; + persistProvider(fallback); + set({ providers, selectedProvider: fallback }); + } else { + set({ providers }); + } + }, + setProvidersLoading: (providersLoading) => set({ providersLoading }), + setSelectedProvider: (providerId, persist = true) => { + if (persist) { + persistProvider(providerId); + } + set({ selectedProvider: providerId }); + }, + // Active agent setActiveAgent: (id) => set({ activeAgentId: id }), diff --git a/ui/goose2/src/features/chat/stores/chatStore.ts b/ui/goose2/src/features/chat/stores/chatStore.ts index d6bab1cad5..066774e8e7 100644 --- a/ui/goose2/src/features/chat/stores/chatStore.ts +++ b/ui/goose2/src/features/chat/stores/chatStore.ts @@ -78,7 +78,14 @@ export const useChatStore = create((set, get) => ({ isConnected: false, // Session management - setActiveSession: (sessionId) => set({ activeSessionId: sessionId }), + setActiveSession: (sessionId) => + set((state) => ({ + activeSessionId: sessionId, + chatState: state.activeSessionId !== sessionId ? "idle" : state.chatState, + streamingMessageId: + state.activeSessionId !== sessionId ? null : state.streamingMessageId, + error: state.activeSessionId !== sessionId ? null : state.error, + })), // Message management addMessage: (sessionId, message) => diff --git a/ui/goose2/src/features/chat/ui/ChatInput.tsx b/ui/goose2/src/features/chat/ui/ChatInput.tsx index 9153a83a5b..c419a51634 100644 --- a/ui/goose2/src/features/chat/ui/ChatInput.tsx +++ b/ui/goose2/src/features/chat/ui/ChatInput.tsx @@ -42,6 +42,7 @@ interface ChatInputProps { onCreatePersona?: () => void; // Provider (secondary -- auto-set by persona but overridable) providers?: AcpProvider[]; + providersLoading?: boolean; selectedProvider?: string; onProviderChange?: (providerId: string) => void; // Model @@ -79,6 +80,7 @@ export function ChatInput({ onPersonaChange, onCreatePersona, providers = [], + providersLoading = false, selectedProvider = "goose", onProviderChange, currentModel = "Claude Sonnet 4", @@ -256,6 +258,7 @@ export function ChatInput({ onPersonaChange={onPersonaChange} onCreatePersona={onCreatePersona} providers={providers} + providersLoading={providersLoading} selectedProvider={selectedProvider} onProviderChange={(id) => onProviderChange?.(id)} currentModel={currentModel} diff --git a/ui/goose2/src/features/chat/ui/ChatInputSelector.tsx b/ui/goose2/src/features/chat/ui/ChatInputSelector.tsx index fda636f3b6..458978c100 100644 --- a/ui/goose2/src/features/chat/ui/ChatInputSelector.tsx +++ b/ui/goose2/src/features/chat/ui/ChatInputSelector.tsx @@ -36,6 +36,7 @@ interface ChatInputSelectorProps { sections: ChatInputSelectorSection[]; onValueChange: (value: string) => void; contentWidth?: "trigger" | "wide"; + disabled?: boolean; } export function ChatInputSelector({ @@ -50,6 +51,7 @@ export function ChatInputSelector({ sections, onValueChange, contentWidth = "trigger", + disabled, }: ChatInputSelectorProps) { const buttonSize = triggerSize === "sm" ? "xs" : "sm"; @@ -62,6 +64,7 @@ export function ChatInputSelector({ size={buttonSize} aria-label={ariaLabel} title={triggerTitle} + disabled={disabled} className={cn( "min-w-0", triggerVariant === "default" && "justify-between", diff --git a/ui/goose2/src/features/chat/ui/ChatInputToolbar.tsx b/ui/goose2/src/features/chat/ui/ChatInputToolbar.tsx index 00f88d8b26..d621b6719c 100644 --- a/ui/goose2/src/features/chat/ui/ChatInputToolbar.tsx +++ b/ui/goose2/src/features/chat/ui/ChatInputToolbar.tsx @@ -29,18 +29,6 @@ const CREATE_PROJECT_VALUE = "__create_project__"; const CREATE_PROJECT_FROM_FOLDER_VALUE = "__create_project_from_folder__"; function formatProviderLabel(providerId: string) { - const knownLabels: Record = { - goose: "Goose", - openai: "OpenAI", - claude: "Claude", - ollama: "Ollama", - custom: "Custom", - }; - - if (knownLabels[providerId]) { - return knownLabels[providerId]; - } - return providerId .split(/[-_\s]+/) .filter(Boolean) @@ -56,6 +44,7 @@ interface ChatInputToolbarProps { onCreatePersona?: () => void; // Provider providers: AcpProvider[]; + providersLoading?: boolean; selectedProvider: string; onProviderChange: (providerId: string) => void; // Model @@ -90,6 +79,7 @@ export function ChatInputToolbar({ onPersonaChange, onCreatePersona, providers, + providersLoading, selectedProvider, onProviderChange, currentModel, @@ -150,14 +140,15 @@ export function ChatInputToolbar({
{/* Left side: pickers */}
- {availableProviderItems.length > 0 && ( + {(availableProviderItems.length > 0 || providersLoading) && ( ({ diff --git a/ui/goose2/src/features/chat/ui/ChatView.tsx b/ui/goose2/src/features/chat/ui/ChatView.tsx index 25e3d5cd19..8280e969a1 100644 --- a/ui/goose2/src/features/chat/ui/ChatView.tsx +++ b/ui/goose2/src/features/chat/ui/ChatView.tsx @@ -5,8 +5,8 @@ import { LoadingGoose } from "./LoadingGoose"; import { useChat } from "../hooks/useChat"; import { useAcpStream } from "../hooks/useAcpStream"; import { useChatStore } from "../stores/chatStore"; -import { discoverAcpProviders, type AcpProvider } from "@/shared/api/acp"; import { useAgentStore } from "@/features/agents/stores/agentStore"; +import { useProviderSelection } from "@/features/agents/hooks/useProviderSelection"; import { useChatSessionStore } from "../stores/chatSessionStore"; import { getProject, type ProjectInfo } from "@/features/projects/api/projects"; import { useProjectStore } from "@/features/projects/stores/projectStore"; @@ -45,7 +45,14 @@ export function ChatView({ onCreateProjectFromFolder, }: ChatViewProps) { const [activeSessionId] = useState(() => sessionId ?? crypto.randomUUID()); - const [providers, setProviders] = useState([]); + + // Provider state from shared store + const { + providers, + providersLoading, + selectedProvider: globalSelectedProvider, + setSelectedProvider: setGlobalSelectedProvider, + } = useProviderSelection(); // Persona state const personas = useAgentStore((s) => s.personas); @@ -76,12 +83,12 @@ export function ChatView({ })), [projects], ); - const effectiveProvider = + // For existing sessions, use their saved provider; otherwise use global selection + const selectedProvider = session?.providerId ?? initialProvider ?? project?.preferredProvider ?? - "goose"; - const [selectedProvider, setSelectedProvider] = useState(effectiveProvider); + globalSelectedProvider; const selectedPersona = personas.find((p) => p.id === selectedPersonaId); const projectFolders = useMemo( @@ -123,41 +130,18 @@ export function ChatView({ }; }, [session?.projectId, storedProject]); - useEffect(() => { - discoverAcpProviders() - .then((discovered) => { - setProviders(discovered); - setSelectedProvider((current) => { - if ( - discovered.length > 0 && - !discovered.some((p) => p.id === current) - ) { - return discovered[0].id; - } - return current; - }); - }) - .catch(() => setProviders([])); - }, []); - - useEffect(() => { - setSelectedProvider((current) => - current === effectiveProvider ? current : effectiveProvider, - ); - }, [effectiveProvider]); - const handleProviderChange = useCallback( (providerId: string) => { if (providerId === selectedProvider) { return; } - setSelectedProvider(providerId); + setGlobalSelectedProvider(providerId); useChatSessionStore .getState() .updateSession(activeSessionId, { providerId }); }, - [activeSessionId, selectedProvider], + [activeSessionId, selectedProvider, setGlobalSelectedProvider], ); const handleProjectChange = useCallback( @@ -342,6 +326,7 @@ export function ChatView({ onCreatePersona={handleCreatePersona} // Providers (secondary) providers={providers} + providersLoading={providersLoading} selectedProvider={selectedProvider} onProviderChange={handleProviderChange} selectedProjectId={session?.projectId ?? null} diff --git a/ui/goose2/src/features/home/ui/HomeScreen.test.tsx b/ui/goose2/src/features/home/ui/HomeScreen.test.tsx index ee74df8880..327e2e918a 100644 --- a/ui/goose2/src/features/home/ui/HomeScreen.test.tsx +++ b/ui/goose2/src/features/home/ui/HomeScreen.test.tsx @@ -1,7 +1,11 @@ import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; import { describe, it, expect, vi, beforeEach } from "vitest"; import { HomeScreen } from "./HomeScreen"; +const setSelectedProvider = vi.fn(); +const setSelectedProviderWithoutPersist = vi.fn(); + vi.mock("@/shared/api/acp", () => ({ discoverAcpProviders: vi.fn().mockResolvedValue([ { id: "goose", label: "Goose" }, @@ -9,6 +13,19 @@ vi.mock("@/shared/api/acp", () => ({ ]), })); +vi.mock("@/features/agents/hooks/useProviderSelection", () => ({ + useProviderSelection: () => ({ + providers: [ + { id: "goose", label: "Goose" }, + { id: "openai", label: "OpenAI" }, + ], + providersLoading: false, + selectedProvider: "goose", + setSelectedProvider, + setSelectedProviderWithoutPersist, + }), +})); + // HomeScreen now reads personas from the agent store, not from ACP providers vi.mock("@/features/agents/stores/agentStore", async (importOriginal) => { const actual = @@ -25,7 +42,29 @@ vi.mock("@/features/agents/stores/agentStore", async (importOriginal) => { id: "builtin-solo", displayName: "Solo", systemPrompt: "You are Solo.", + provider: "openai", + description: null, + avatar: null, + createdBy: null, + source: "custom", + extensions: [], + metadata: null, + sortOrder: 0, + isDefault: false, + }, + { + id: "builtin-goose", + displayName: "Goosey", + systemPrompt: "You are Goosey.", isBuiltin: true, + description: null, + avatar: null, + createdBy: null, + source: "custom", + extensions: [], + metadata: null, + sortOrder: 1, + isDefault: false, createdAt: "", updatedAt: "", }, @@ -44,6 +83,10 @@ describe("HomeScreen", () => { beforeEach(() => { vi.useFakeTimers(); vi.setSystemTime(new Date(2026, 2, 29, 14, 30, 0)); // 2:30 PM + localStorage.clear(); + localStorage.setItem("goose:defaultProvider", "goose"); + setSelectedProvider.mockReset(); + setSelectedProviderWithoutPersist.mockReset(); }); it("renders the clock", () => { @@ -80,4 +123,24 @@ describe("HomeScreen", () => { screen.getByRole("button", { name: /select project/i }), ).toBeInTheDocument(); }); + + it("reverts to the stored provider when a persona override is cleared", async () => { + vi.useRealTimers(); + const user = userEvent.setup(); + + render(); + + await user.click(screen.getByRole("button", { name: /choose assistant/i })); + await user.click(screen.getByRole("menuitem", { name: /solo/i })); + + expect(setSelectedProviderWithoutPersist).toHaveBeenLastCalledWith( + "openai", + ); + + await user.click( + screen.getByRole("button", { name: /clear active assistant/i }), + ); + + expect(setSelectedProviderWithoutPersist).toHaveBeenLastCalledWith("goose"); + }); }); diff --git a/ui/goose2/src/features/home/ui/HomeScreen.tsx b/ui/goose2/src/features/home/ui/HomeScreen.tsx index de0d63630c..7726eff75e 100644 --- a/ui/goose2/src/features/home/ui/HomeScreen.tsx +++ b/ui/goose2/src/features/home/ui/HomeScreen.tsx @@ -1,8 +1,11 @@ import { useState, useEffect, useCallback } from "react"; -import { useAgentStore } from "@/features/agents/stores/agentStore"; +import { + getStoredProvider, + useAgentStore, +} from "@/features/agents/stores/agentStore"; +import { useProviderSelection } from "@/features/agents/hooks/useProviderSelection"; import { ChatInput } from "@/features/chat/ui/ChatInput"; import { useProjectStore } from "@/features/projects/stores/projectStore"; -import { discoverAcpProviders, type AcpProvider } from "@/shared/api/acp"; function HomeClock() { const [time, setTime] = useState(new Date()); @@ -60,37 +63,33 @@ export function HomeScreen({ const greeting = getGreeting(hour); const personas = useAgentStore((s) => s.personas); + const { + providers, + providersLoading, + selectedProvider, + setSelectedProvider, + setSelectedProviderWithoutPersist, + } = useProviderSelection(); const projects = useProjectStore((s) => s.projects); const [selectedPersonaId, setSelectedPersonaId] = useState( null, ); - const [providers, setProviders] = useState([]); - const [selectedProvider, setSelectedProvider] = useState("goose"); const [selectedProjectId, setSelectedProjectId] = useState( null, ); - const selectedPersona = personas.find((p) => p.id === selectedPersonaId); - useEffect(() => { - discoverAcpProviders() - .then((discovered) => { - setProviders(discovered); - setSelectedProvider((current) => { - if ( - discovered.length > 0 && - !discovered.some((provider) => provider.id === current) - ) { - return discovered[0].id; - } - return current; - }); - }) - .catch(() => setProviders([])); - }, []); + const handlePersonaChange = useCallback( + (personaId: string | null) => { + setSelectedPersonaId(personaId); + const persona = personaId + ? personas.find((candidate) => candidate.id === personaId) + : null; + const nextProvider = persona?.provider ?? getStoredProvider(providers); - useEffect(() => { - setSelectedProvider(selectedPersona?.provider ?? "goose"); - }, [selectedPersona?.provider]); + setSelectedProviderWithoutPersist(nextProvider); + }, + [personas, providers, setSelectedProviderWithoutPersist], + ); const handleCreatePersona = useCallback(() => { useAgentStore.getState().openPersonaEditor(); @@ -127,9 +126,10 @@ export function HomeScreen({ onSend={handleSend} personas={personas} selectedPersonaId={selectedPersonaId} - onPersonaChange={setSelectedPersonaId} + onPersonaChange={handlePersonaChange} onCreatePersona={handleCreatePersona} providers={providers} + providersLoading={providersLoading} selectedProvider={selectedProvider} onProviderChange={setSelectedProvider} selectedProjectId={selectedProjectId}