From b4c08793a980ce566dec5c8d3958c66da16e0ce9 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Thu, 30 Apr 2026 04:13:10 +1000 Subject: [PATCH] fix: model picker stays usable during provider loading (#8900) Co-authored-by: Claude Opus 4.6 (1M context) Co-authored-by: Kalvin Chau --- ui/goose2/src/app/hooks/useAppStartup.ts | 50 +++------- .../chat/hooks/useAgentModelPickerState.ts | 22 ++++- .../chat/hooks/useChatSessionController.ts | 2 + .../chat/hooks/useResolvedAgentModelPicker.ts | 2 + ui/goose2/src/features/chat/types.ts | 1 + .../src/features/chat/ui/AgentModelPicker.tsx | 72 ++++++-------- ui/goose2/src/features/chat/ui/ChatInput.tsx | 2 + .../src/features/chat/ui/ChatInputToolbar.tsx | 3 + ui/goose2/src/features/chat/ui/ChatView.tsx | 1 + ui/goose2/src/features/home/ui/HomeScreen.tsx | 1 + .../features/providers/api/inventory.test.ts | 97 +++++++++++++++++++ .../src/features/providers/api/inventory.ts | 40 ++++++++ 12 files changed, 215 insertions(+), 78 deletions(-) create mode 100644 ui/goose2/src/features/providers/api/inventory.test.ts diff --git a/ui/goose2/src/app/hooks/useAppStartup.ts b/ui/goose2/src/app/hooks/useAppStartup.ts index ed4dd5fb..78b0cc7e 100644 --- a/ui/goose2/src/app/hooks/useAppStartup.ts +++ b/ui/goose2/src/app/hooks/useAppStartup.ts @@ -75,40 +75,6 @@ export function useAppStartup() { } }; - const refreshConfiguredProviderInventory = async ( - initialEntries?: Awaited>, - ) => { - try { - const entries = - initialEntries && initialEntries.length > 0 - ? initialEntries - : await (async () => { - const { getProviderInventory } = await import( - "@/features/providers/api/inventory" - ); - return getProviderInventory(); - })(); - const configuredProviderIds = entries - .filter((entry) => entry.configured) - .map((entry) => entry.providerId); - if (configuredProviderIds.length === 0) { - return; - } - - const { syncProviderInventory } = await import( - "@/features/providers/api/inventorySync" - ); - await syncProviderInventory(configuredProviderIds, { - onEntries: (entries) => inventoryStore.mergeEntries(entries), - }); - } catch (err) { - console.error( - "Failed to refresh provider inventory on startup:", - err, - ); - } - }; - const loadSessionState = async () => { const t0 = performance.now(); perfLog("[perf:startup] loadSessionState start"); @@ -128,9 +94,19 @@ export function useAppStartup() { providersAndInventoryLoad, loadSessionState(), ]); - void providersAndInventoryLoad.then((entries) => - refreshConfiguredProviderInventory(entries), - ); + void providersAndInventoryLoad.then(async (entries) => { + try { + const { backgroundRefreshInventory } = await import( + "@/features/providers/api/inventory" + ); + await backgroundRefreshInventory(inventoryStore, entries); + } catch (err) { + console.error( + "Failed to refresh provider inventory on startup:", + err, + ); + } + }); perfLog( `[perf:startup] useAppStartup complete in ${(performance.now() - tStartup).toFixed(1)}ms`, ); diff --git a/ui/goose2/src/features/chat/hooks/useAgentModelPickerState.ts b/ui/goose2/src/features/chat/hooks/useAgentModelPickerState.ts index 6534c9da..f5f5fc47 100644 --- a/ui/goose2/src/features/chat/hooks/useAgentModelPickerState.ts +++ b/ui/goose2/src/features/chat/hooks/useAgentModelPickerState.ts @@ -1,6 +1,7 @@ -import { useCallback, useMemo } from "react"; +import { useCallback, useMemo, useRef } from "react"; import type { AcpProvider } from "@/shared/api/acp"; import { useProviderInventory } from "@/features/providers/hooks/useProviderInventory"; +import { useProviderInventoryStore } from "@/features/providers/stores/providerInventoryStore"; import { getCatalogEntry, resolveAgentProviderCatalogIdStrict, @@ -162,6 +163,24 @@ export function useAgentModelPickerState({ [availableModels, onModelSelected], ); + const refreshingRef = useRef(false); + const handlePickerOpen = useCallback(() => { + if (refreshingRef.current || useProviderInventoryStore.getState().loading) { + return; + } + refreshingRef.current = true; + import("@/features/providers/api/inventory") + .then(({ backgroundRefreshInventory }) => + backgroundRefreshInventory(useProviderInventoryStore.getState()), + ) + .catch((err) => + console.error("Failed to background-refresh inventory:", err), + ) + .finally(() => { + refreshingRef.current = false; + }); + }, []); + return { selectedAgentId, pickerAgents, @@ -170,5 +189,6 @@ export function useAgentModelPickerState({ modelStatusMessage, handleProviderChange, handleModelChange, + handlePickerOpen, }; } diff --git a/ui/goose2/src/features/chat/hooks/useChatSessionController.ts b/ui/goose2/src/features/chat/hooks/useChatSessionController.ts index 7c04e6ea..9db7e11c 100644 --- a/ui/goose2/src/features/chat/hooks/useChatSessionController.ts +++ b/ui/goose2/src/features/chat/hooks/useChatSessionController.ts @@ -230,6 +230,7 @@ export function useChatSessionController({ modelStatusMessage, handleProviderChange, handleModelChange, + handlePickerOpen, effectiveModelSelection, } = useResolvedAgentModelPicker({ providers, @@ -819,6 +820,7 @@ export function useChatSessionController({ modelsLoading, modelStatusMessage, handleModelChange: handleModelChangeWithContextReset, + handlePickerOpen, selectedProjectId: effectiveProjectId, availableProjects, handleProjectChange, diff --git a/ui/goose2/src/features/chat/hooks/useResolvedAgentModelPicker.ts b/ui/goose2/src/features/chat/hooks/useResolvedAgentModelPicker.ts index 51353300..a1638936 100644 --- a/ui/goose2/src/features/chat/hooks/useResolvedAgentModelPicker.ts +++ b/ui/goose2/src/features/chat/hooks/useResolvedAgentModelPicker.ts @@ -184,6 +184,7 @@ export function useResolvedAgentModelPicker({ modelStatusMessage, handleProviderChange, handleModelChange, + handlePickerOpen, } = useAgentModelPickerState({ providers, selectedProvider, @@ -463,6 +464,7 @@ export function useResolvedAgentModelPicker({ modelStatusMessage, handleProviderChange, handleModelChange, + handlePickerOpen, effectiveModelSelection, }; } diff --git a/ui/goose2/src/features/chat/types.ts b/ui/goose2/src/features/chat/types.ts index 3472253b..1e35c460 100644 --- a/ui/goose2/src/features/chat/types.ts +++ b/ui/goose2/src/features/chat/types.ts @@ -64,6 +64,7 @@ export interface ChatInputProps { modelsLoading?: boolean; modelStatusMessage?: string | null; onModelChange?: (modelId: string) => void; + onPickerOpen?: () => void; selectedProjectId?: string | null; availableProjects?: ProjectOption[]; onProjectChange?: (projectId: string | null) => void; diff --git a/ui/goose2/src/features/chat/ui/AgentModelPicker.tsx b/ui/goose2/src/features/chat/ui/AgentModelPicker.tsx index 43ad4ec6..99148839 100644 --- a/ui/goose2/src/features/chat/ui/AgentModelPicker.tsx +++ b/ui/goose2/src/features/chat/ui/AgentModelPicker.tsx @@ -7,8 +7,6 @@ import { } from "@tabler/icons-react"; import { useTranslation } from "react-i18next"; import type { AcpProvider } from "@/shared/api/acp"; -import { getProviderInventory } from "@/features/providers/api/inventory"; -import { useProviderInventoryStore } from "@/features/providers/stores/providerInventoryStore"; import { cn } from "@/shared/lib/cn"; import { Button } from "@/shared/ui/button"; import { Popover, PopoverContent, PopoverTrigger } from "@/shared/ui/popover"; @@ -34,6 +32,7 @@ interface AgentModelPickerProps { loading?: boolean; isCompact?: boolean; showSelectedModelInTrigger?: boolean; + onOpen?: () => void; } function getModelDisplayName(model: ModelOption) { @@ -321,14 +320,11 @@ export function AgentModelPicker({ loading = false, isCompact = false, showSelectedModelInTrigger = true, + onOpen, }: AgentModelPickerProps) { const { t } = useTranslation("chat"); const [open, setOpen] = useState(false); const [modelView, setModelView] = useState("recommended"); - const mergeInventoryEntries = useProviderInventoryStore( - (s) => s.mergeEntries, - ); - const selectedAgentLabel = agents.find((agent) => agent.id === selectedAgentId)?.label ?? formatProviderLabel(selectedAgentId); @@ -358,52 +354,32 @@ export function AgentModelPicker({ } }, [open]); - useEffect(() => { - if (!open) { - return; - } - - let cancelled = false; - - const syncInventory = async () => { - try { - const entries = await getProviderInventory(); - if (cancelled) { - return; - } - mergeInventoryEntries(entries); - } catch (error) { - console.error("Failed to sync provider inventory from picker:", error); - } - }; - - void syncInventory(); - - return () => { - cancelled = true; - }; - }, [open, mergeInventoryEntries]); - // When in "all" view, expand the popover to full width for the search experience. const isAllView = modelView === "all"; return ( - + { + setOpen(nextOpen); + if (nextOpen) onOpen?.(); + }} + > @@ -512,9 +488,25 @@ export function AgentModelPicker({ className="flex min-h-0 min-w-0 overflow-hidden p-1" > {modelsLoading ? ( -
- - {t("toolbar.loadingModels")} +
+
+ {t("toolbar.model")} +
+ {currentModelName || currentModelId ? ( +
+ +
+ {currentModelName ?? currentModelId} +
+ + +
+ ) : ( +
+ + {t("toolbar.loadingModels")} +
+ )}
) : availableModels.length > 0 ? ( modelView === "recommended" ? ( diff --git a/ui/goose2/src/features/chat/ui/ChatInput.tsx b/ui/goose2/src/features/chat/ui/ChatInput.tsx index 438b1494..51feb03f 100644 --- a/ui/goose2/src/features/chat/ui/ChatInput.tsx +++ b/ui/goose2/src/features/chat/ui/ChatInput.tsx @@ -48,6 +48,7 @@ export function ChatInput({ modelsLoading = false, modelStatusMessage = null, onModelChange, + onPickerOpen, selectedProjectId = null, availableProjects = [], onProjectChange, @@ -455,6 +456,7 @@ export function ChatInput({ modelsLoading={modelsLoading} modelStatusMessage={modelStatusMessage} onModelChange={onModelChange} + onPickerOpen={onPickerOpen} selectedProjectId={selectedProjectId} availableProjects={availableProjects} onProjectChange={onProjectChange} diff --git a/ui/goose2/src/features/chat/ui/ChatInputToolbar.tsx b/ui/goose2/src/features/chat/ui/ChatInputToolbar.tsx index 741fb8af..1670e951 100644 --- a/ui/goose2/src/features/chat/ui/ChatInputToolbar.tsx +++ b/ui/goose2/src/features/chat/ui/ChatInputToolbar.tsx @@ -63,6 +63,7 @@ interface ChatInputToolbarProps { modelsLoading?: boolean; modelStatusMessage?: string | null; onModelChange?: (modelId: string) => void; + onPickerOpen?: () => void; // Project selectedProjectId: string | null; availableProjects: ProjectOption[]; @@ -108,6 +109,7 @@ export function ChatInputToolbar({ modelsLoading = false, modelStatusMessage = null, onModelChange, + onPickerOpen, selectedProjectId, availableProjects, onProjectChange, @@ -231,6 +233,7 @@ export function ChatInputToolbar({ modelsLoading={modelsLoading} modelStatusMessage={modelStatusMessage} onModelChange={onModelChange} + onOpen={onPickerOpen} loading={providersLoading} isCompact={isCompact} showSelectedModelInTrigger={selectedPersonaId === null} diff --git a/ui/goose2/src/features/chat/ui/ChatView.tsx b/ui/goose2/src/features/chat/ui/ChatView.tsx index 3d93ecbb..b5041f94 100644 --- a/ui/goose2/src/features/chat/ui/ChatView.tsx +++ b/ui/goose2/src/features/chat/ui/ChatView.tsx @@ -139,6 +139,7 @@ export function ChatView({ modelsLoading={controller.modelsLoading} modelStatusMessage={controller.modelStatusMessage} onModelChange={controller.handleModelChange} + onPickerOpen={controller.handlePickerOpen} selectedProjectId={controller.selectedProjectId} availableProjects={controller.availableProjects} onProjectChange={controller.handleProjectChange} diff --git a/ui/goose2/src/features/home/ui/HomeScreen.tsx b/ui/goose2/src/features/home/ui/HomeScreen.tsx index 55dc96f7..1b8e9c61 100644 --- a/ui/goose2/src/features/home/ui/HomeScreen.tsx +++ b/ui/goose2/src/features/home/ui/HomeScreen.tsx @@ -92,6 +92,7 @@ function HomeComposer({ modelsLoading={controller.modelsLoading} modelStatusMessage={controller.modelStatusMessage} onModelChange={controller.handleModelChange} + onPickerOpen={controller.handlePickerOpen} selectedProjectId={controller.selectedProjectId} availableProjects={controller.availableProjects} onProjectChange={controller.handleProjectChange} diff --git a/ui/goose2/src/features/providers/api/inventory.test.ts b/ui/goose2/src/features/providers/api/inventory.test.ts new file mode 100644 index 00000000..daafbcce --- /dev/null +++ b/ui/goose2/src/features/providers/api/inventory.test.ts @@ -0,0 +1,97 @@ +import type { ProviderInventoryEntryDto } from "@aaif/goose-sdk"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { backgroundRefreshInventory } from "./inventory"; + +const mockClient = vi.hoisted(() => ({ + GooseProvidersList: vi.fn(), + GooseProvidersInventoryRefresh: vi.fn(), +})); + +vi.mock("@/shared/api/acpConnection", () => ({ + getClient: vi.fn(async () => ({ + goose: mockClient, + })), +})); + +function providerEntry( + overrides: Partial, +): ProviderInventoryEntryDto { + return { + providerId: "openai", + providerName: "OpenAI", + description: "", + defaultModel: "", + configured: false, + providerType: "Preferred", + configKeys: [], + setupSteps: [], + supportsRefresh: false, + refreshing: false, + models: [], + stale: false, + ...overrides, + }; +} + +describe("backgroundRefreshInventory", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("merges fetched inventory before returning when no providers are configured", async () => { + const entries = [ + providerEntry({ providerId: "openai", providerName: "OpenAI" }), + ]; + const inventoryStore = { mergeEntries: vi.fn() }; + mockClient.GooseProvidersList.mockResolvedValue({ entries }); + + await backgroundRefreshInventory(inventoryStore); + + expect(inventoryStore.mergeEntries).toHaveBeenCalledWith(entries); + expect(mockClient.GooseProvidersInventoryRefresh).not.toHaveBeenCalled(); + }); + + it("merges fetched inventory before returning when no refresh starts", async () => { + const entries = [ + providerEntry({ + providerId: "openai", + providerName: "OpenAI", + configured: true, + }), + ]; + const inventoryStore = { mergeEntries: vi.fn() }; + mockClient.GooseProvidersList.mockResolvedValue({ entries }); + mockClient.GooseProvidersInventoryRefresh.mockResolvedValue({ + started: [], + }); + + await backgroundRefreshInventory(inventoryStore); + + expect(inventoryStore.mergeEntries).toHaveBeenCalledWith(entries); + expect(mockClient.GooseProvidersInventoryRefresh).toHaveBeenCalledWith({ + providerIds: ["openai"], + }); + }); + + it("does not re-merge entries supplied by a caller that already stored them", async () => { + const entries = [ + providerEntry({ + providerId: "openai", + providerName: "OpenAI", + configured: true, + }), + ]; + const inventoryStore = { mergeEntries: vi.fn() }; + mockClient.GooseProvidersInventoryRefresh.mockResolvedValue({ + started: [], + }); + + await backgroundRefreshInventory(inventoryStore, entries); + + expect(mockClient.GooseProvidersList).not.toHaveBeenCalled(); + expect(inventoryStore.mergeEntries).not.toHaveBeenCalled(); + expect(mockClient.GooseProvidersInventoryRefresh).toHaveBeenCalledWith({ + providerIds: ["openai"], + }); + }); +}); diff --git a/ui/goose2/src/features/providers/api/inventory.ts b/ui/goose2/src/features/providers/api/inventory.ts index 74d10129..39fcee00 100644 --- a/ui/goose2/src/features/providers/api/inventory.ts +++ b/ui/goose2/src/features/providers/api/inventory.ts @@ -30,3 +30,43 @@ export async function refreshProviderInventory( ); return response; } + +/** + * Refresh configured provider inventories in the background, polling until + * all providers finish refreshing. If no entries are supplied, fetch and merge + * the current inventory snapshot first so the UI sees fresh cached data even + * when no refresh starts. + * + * Does NOT set the store's `loading` flag, so the UI keeps showing cached data + * during the refresh. + */ +export async function backgroundRefreshInventory( + inventoryStore: { + mergeEntries: (entries: ProviderInventoryEntryDto[]) => void; + }, + initialEntries?: ProviderInventoryEntryDto[], +): Promise { + const entries = initialEntries?.length + ? initialEntries + : await getProviderInventory(); + + if (!initialEntries?.length) { + inventoryStore.mergeEntries(entries); + } + + const configuredProviderIds = entries + .filter((entry) => entry.configured) + .map((entry) => entry.providerId); + if (configuredProviderIds.length === 0) return; + + const refresh = await refreshProviderInventory(configuredProviderIds); + if (refresh.started.length === 0 && (refresh.skipped ?? []).length === 0) { + return; + } + + const { syncProviderInventory } = await import("./inventorySync"); + await syncProviderInventory(configuredProviderIds, { + initialRefresh: refresh, + onEntries: (entries) => inventoryStore.mergeEntries(entries), + }); +}