From d2f0c0ee27fb490f54eb11d54401879a1a9ed08b Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Thu, 3 Sep 2026 11:20:31 -0400 Subject: [PATCH 01/24] feat(chat): add starred models to the agent model picker Star/unstar any model from the agent model picker. Starred models surface in a Starred section pinned at the top of the picker lists. Persistence lives in a small lib + hook: localStorage key goose:starredModels:v1, keyed per (scopeId, modelId), with a window event to sync pickers across windows. Adds en/es strings for star/unstar actions and extends the picker test suite (45 tests). Co-authored-by: Goose --- src/features/chat/hooks/useStarredModels.ts | 60 ++++++ src/features/chat/lib/starredModels.ts | 69 ++++++ src/features/chat/ui/AgentModelPicker.tsx | 8 +- .../chat/ui/AgentModelPickerLists.tsx | 200 +++++++++++++----- .../ui/__tests__/AgentModelPicker.test.tsx | 134 +++++++++++- .../design-system/ui/designSystemSections.ts | 2 +- src/shared/i18n/locales/en/chat.json | 4 +- src/shared/i18n/locales/es/chat.json | 4 +- 8 files changed, 417 insertions(+), 64 deletions(-) create mode 100644 src/features/chat/hooks/useStarredModels.ts create mode 100644 src/features/chat/lib/starredModels.ts diff --git a/src/features/chat/hooks/useStarredModels.ts b/src/features/chat/hooks/useStarredModels.ts new file mode 100644 index 000000000..843a93628 --- /dev/null +++ b/src/features/chat/hooks/useStarredModels.ts @@ -0,0 +1,60 @@ +import { useCallback, useSyncExternalStore } from "react"; +import { + getStarredModelKeys, + modelStarKey, + STARRED_MODELS_EVENT, + STARRED_MODELS_KEY, + toggleModelStar, +} from "../lib/starredModels"; + +let cachedSnapshot: Set | null = null; +const serverSnapshot = new Set(); + +/** Invalidate the in-memory snapshot cache. Intended for tests. */ +export function __resetStarredModelsCacheForTests(): void { + cachedSnapshot = null; +} + +function getSnapshot(): Set { + if (cachedSnapshot === null) { + cachedSnapshot = getStarredModelKeys(); + } + return cachedSnapshot; +} + +function subscribe(callback: () => void): () => void { + const handleChange = () => { + cachedSnapshot = null; + callback(); + }; + const handleStorage = (event: StorageEvent) => { + if (event.key === null || event.key === STARRED_MODELS_KEY) { + handleChange(); + } + }; + + window.addEventListener(STARRED_MODELS_EVENT, handleChange); + window.addEventListener("storage", handleStorage); + return () => { + window.removeEventListener(STARRED_MODELS_EVENT, handleChange); + window.removeEventListener("storage", handleStorage); + }; +} + +export function useStarredModels() { + const starredKeys = useSyncExternalStore( + subscribe, + getSnapshot, + () => serverSnapshot, + ); + const isStarred = useCallback( + (scopeId: string, modelId: string) => + starredKeys.has(modelStarKey(scopeId, modelId)), + [starredKeys], + ); + const toggleStar = useCallback((scopeId: string, modelId: string) => { + toggleModelStar(scopeId, modelId); + }, []); + + return { isStarred, toggleStar, starredKeys }; +} diff --git a/src/features/chat/lib/starredModels.ts b/src/features/chat/lib/starredModels.ts new file mode 100644 index 000000000..c49fcdf86 --- /dev/null +++ b/src/features/chat/lib/starredModels.ts @@ -0,0 +1,69 @@ +const STARRED_MODELS_STORAGE_KEY = "goose:starredModels:v1"; +const STARRED_MODELS_CHANGED_EVENT = "goose:starred-models-changed"; + +type StarredModelSet = Set; + +export function modelStarKey(scopeId: string, modelId: string): string { + return JSON.stringify([scopeId, modelId]); +} + +function readStarredModels(): StarredModelSet { + if (typeof window === "undefined") { + return new Set(); + } + + try { + const stored = window.localStorage.getItem(STARRED_MODELS_STORAGE_KEY); + if (!stored) { + return new Set(); + } + + const parsed = JSON.parse(stored); + if (!Array.isArray(parsed)) { + return new Set(); + } + + return new Set(parsed.filter((item) => typeof item === "string")); + } catch { + return new Set(); + } +} + +function persistStarredModels(models: StarredModelSet): void { + if (typeof window === "undefined") { + return; + } + + try { + if (models.size === 0) { + window.localStorage.removeItem(STARRED_MODELS_STORAGE_KEY); + } else { + window.localStorage.setItem( + STARRED_MODELS_STORAGE_KEY, + JSON.stringify([...models]), + ); + } + } catch { + // localStorage may be unavailable. + } + + window.dispatchEvent(new CustomEvent(STARRED_MODELS_CHANGED_EVENT)); +} + +export function getStarredModelKeys(): StarredModelSet { + return readStarredModels(); +} + +export function toggleModelStar(scopeId: string, modelId: string): void { + const next = readStarredModels(); + const key = modelStarKey(scopeId, modelId); + if (next.has(key)) { + next.delete(key); + } else { + next.add(key); + } + persistStarredModels(next); +} + +export const STARRED_MODELS_EVENT = STARRED_MODELS_CHANGED_EVENT; +export const STARRED_MODELS_KEY = STARRED_MODELS_STORAGE_KEY; diff --git a/src/features/chat/ui/AgentModelPicker.tsx b/src/features/chat/ui/AgentModelPicker.tsx index 2adfbb91d..313c197c0 100644 --- a/src/features/chat/ui/AgentModelPicker.tsx +++ b/src/features/chat/ui/AgentModelPicker.tsx @@ -79,8 +79,8 @@ type PopoverContentAlign = NonNullable< ComponentProps["align"] >; const REASONING_EFFORT_COLUMN_TRANSITION_MS = 240; -const PICKER_WIDTH_COMPACT_PX = 420; -const PICKER_WIDTH_EXPANDED_PX = 596; +const PICKER_WIDTH_COMPACT_PX = 452; +const PICKER_WIDTH_EXPANDED_PX = 628; function toSentenceCaseLabel(value: string | undefined): string { const trimmed = value?.trim(); @@ -500,7 +500,7 @@ export function AgentModelPicker({ // gated single-column layout has no dead vertical space below the // model list. "flex max-h-[min(24rem,50vh)] flex-col overflow-hidden p-1 transition-[width] duration-[240ms] ease-[cubic-bezier(0.2,0,0,1)]", - isWidePicker ? "w-[37.25rem]" : "w-[26.25rem]", + isWidePicker ? "w-[39.25rem]" : "w-[28.25rem]", )} onInteractOutside={(event) => { classifyOutsideInteraction(event.target); @@ -672,7 +672,7 @@ export function AgentModelPicker({ data-col="model" className={cn( "flex min-h-0 min-w-0 overflow-hidden p-1", - showAgentColumn ? "ml-1 w-56 shrink-0" : "flex-1", + showAgentColumn ? "ml-1 w-64 shrink-0" : "flex-1", )} > {modelsLoading ? ( diff --git a/src/features/chat/ui/AgentModelPickerLists.tsx b/src/features/chat/ui/AgentModelPickerLists.tsx index 7ad2902bd..15a129d69 100644 --- a/src/features/chat/ui/AgentModelPickerLists.tsx +++ b/src/features/chat/ui/AgentModelPickerLists.tsx @@ -7,10 +7,20 @@ import { useRef, useState, } from "react"; -import { IconCheck, IconDots, IconSearch, IconX } from "@tabler/icons-react"; +import { + IconCheck, + IconDots, + IconSearch, + IconStar, + IconStarFilled, + IconX, +} from "@tabler/icons-react"; +import { useStarredModels } from "../hooks/useStarredModels"; +import { modelStarKey } from "../lib/starredModels"; import { SearchBar } from "@/shared/ui/SearchBar"; import { Button } from "@/shared/ui/button"; import { ScrollArea } from "@/shared/ui/scroll-area"; +import { Separator } from "@/shared/ui/separator"; import { formatProviderLabel, getProviderIcon, @@ -92,11 +102,18 @@ function sortModels( recency: { map: ModelRecencyMap; agentId: string }, ) { return [...models].sort((left, right) => { - if (modelMatchesSelection(left, currentModelId, currentModelProviderId)) { - return -1; - } - if (modelMatchesSelection(right, currentModelId, currentModelProviderId)) { - return 1; + const leftSelected = modelMatchesSelection( + left, + currentModelId, + currentModelProviderId, + ); + const rightSelected = modelMatchesSelection( + right, + currentModelId, + currentModelProviderId, + ); + if (leftSelected !== rightSelected) { + return leftSelected ? -1 : 1; } const leftRank = getModelRecencyRank(recency.map, recency.agentId, left); @@ -127,7 +144,7 @@ interface ModelListProps { * would interrupt browsing. */ onBrowseChange?: (browsing: boolean) => void; - t: (key: string) => string; + t: (key: string, options?: Record) => string; } export interface RecommendedModelListHandle { @@ -149,6 +166,7 @@ export const RecommendedModelList = forwardRef< }, ref, ) { + const { isStarred, toggleStar, starredKeys } = useStarredModels(); const [searchOpen, setSearchOpen] = useState(false); const [showAll, setShowAll] = useState(false); const [query, setQuery] = useState(""); @@ -172,6 +190,11 @@ export const RecommendedModelList = forwardRef< }, [resetScroll]); const recencyMap = useModelRecency(); const recommended = useMemo(() => { + const starred = models.filter((model) => + starredKeys.has( + modelStarKey(model.providerId ?? selectedAgentId, model.id), + ), + ); const recent = models .map((m) => ({ model: m, @@ -184,6 +207,12 @@ export const RecommendedModelList = forwardRef< entry.model, currentModelId, currentModelProviderId, + ) && + !starredKeys.has( + modelStarKey( + entry.model.providerId ?? selectedAgentId, + entry.model.id, + ), ), ) .sort((left, right) => { @@ -199,30 +228,44 @@ export const RecommendedModelList = forwardRef< .filter((m) => m.recommended) .filter( (m) => - !recent.some((r) => r.id === m.id && r.providerId === m.providerId), + !recent.some((r) => r.id === m.id && r.providerId === m.providerId) && + !starredKeys.has(modelStarKey(m.providerId ?? selectedAgentId, m.id)), ); const shortlist = [...recent, ...rec]; if ( currentModelId && - shortlist.length > 0 && + starred.length + shortlist.length > 0 && + !starred.some((m) => + modelMatchesSelection(m, currentModelId, currentModelProviderId), + ) && !shortlist.some((m) => modelMatchesSelection(m, currentModelId, currentModelProviderId), ) ) { - const current = models.find((m) => - modelMatchesSelection(m, currentModelId, currentModelProviderId), + const current = models.find((model) => + modelMatchesSelection(model, currentModelId, currentModelProviderId), ); if (current) { return [current, ...shortlist]; } } - return shortlist.length > 0 ? shortlist : models; + const unstarredFallback = models.filter( + (model) => + !starredKeys.has( + modelStarKey(model.providerId ?? selectedAgentId, model.id), + ), + ); + return [ + ...starred, + ...(shortlist.length > 0 ? shortlist : unstarredFallback), + ]; }, [ models, currentModelId, currentModelProviderId, recencyMap, selectedAgentId, + starredKeys, ]); useEffect(() => { @@ -265,22 +308,47 @@ export const RecommendedModelList = forwardRef< ); }, [models, query, recommended, searchOpen, showAll]); - const sorted = useMemo( - () => - sortModels(visibleModels, currentModelId, currentModelProviderId, { + const grouped = useMemo(() => { + const starred: ModelOption[] = []; + const unstarred: ModelOption[] = []; + for (const model of visibleModels) { + const scopeId = model.providerId ?? selectedAgentId; + (starredKeys.has(modelStarKey(scopeId, model.id)) + ? starred + : unstarred + ).push(model); + } + return { + starred: sortModels(starred, currentModelId, currentModelProviderId, { map: recencyMap, agentId: selectedAgentId, }), - [ - visibleModels, - currentModelId, - currentModelProviderId, - recencyMap, - selectedAgentId, - ], - ); + unstarred: sortModels(unstarred, currentModelId, currentModelProviderId, { + map: recencyMap, + agentId: selectedAgentId, + }), + }; + }, [ + visibleModels, + currentModelId, + currentModelProviderId, + recencyMap, + selectedAgentId, + starredKeys, + ]); + const sorted = [...grouped.starred, ...grouped.unstarred]; - const hasMore = models.length > recommended.length; + const recommendedKeys = new Set( + recommended.map((model) => + modelStarKey(model.providerId ?? selectedAgentId, model.id), + ), + ); + const hasMore = models.some( + (model) => + !recommendedKeys.has( + modelStarKey(model.providerId ?? selectedAgentId, model.id), + ), + ); const showSearchButton = hasMore || recommended.length > SEARCHABLE_LIST_THRESHOLD; const closeSearch = useCallback(() => { @@ -369,7 +437,7 @@ export const RecommendedModelList = forwardRef< className="min-h-0 min-w-0 flex-1 [&_[data-slot=scroll-area-viewport]>div]:!block" >
- {sorted.map((model) => { + {sorted.map((model, index) => { const providerLabel = getGooseModelProviderLabel(model); const providerIcon = selectedAgentId === "goose" && model.providerId @@ -380,33 +448,67 @@ export const RecommendedModelList = forwardRef< currentModelId, currentModelProviderId, ); + const scopeId = model.providerId ?? selectedAgentId; + const starred = isStarred(scopeId, model.id); + const showStarredDivider = + index === grouped.starred.length - 1 && + grouped.unstarred.length > 0; return ( - { - onModelSelect(model); - resetView(); - }} - selected={isSelected} - className="justify-between" - > -
- {providerIcon ? ( - - {providerIcon} - - ) : null} -
- {getModelDisplayName(model)} -
+
+
+ { + onModelSelect(model); + resetView(); + }} + selected={isSelected} + className="w-auto flex-1 justify-between" + > +
+ {providerIcon ? ( + + {providerIcon} + + ) : null} +
+ {getModelDisplayName(model)} +
+
+ {isSelected ? ( + + ) : null} +
+
- {isSelected ? ( - + {showStarredDivider ? ( + ) : null} - +
); })} {hasMore && !searchOpen && !showAll ? ( diff --git a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx index 056f71e6f..c7e375a1d 100644 --- a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx +++ b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx @@ -1,7 +1,9 @@ import type { ComponentProps } from "react"; import { render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { __resetStarredModelsCacheForTests } from "../../hooks/useStarredModels"; +import { modelStarKey, STARRED_MODELS_KEY } from "../../lib/starredModels"; import { AgentModelPicker } from "../AgentModelPicker"; import { getModelRecencyMap, @@ -232,7 +234,7 @@ describe("AgentModelPicker", () => { await user.click(trigger); const explicitModel = screen.getByRole("button", { - name: /Claude Opus 4\.8/, + name: /^Claude Opus 4\.8$/, }); expect(explicitModel).toHaveClass("bg-accent"); expect( @@ -269,7 +271,7 @@ describe("AgentModelPicker", () => { screen.queryByRole("button", { name: /synthetic-model/i }), ).not.toBeInTheDocument(); expect( - screen.getByRole("button", { name: /GPT-5\.5/i }), + screen.getByRole("button", { name: /^GPT-5\.5$/i }), ).toBeInTheDocument(); }); @@ -878,7 +880,7 @@ describe("AgentModelPicker", () => { const picker = screen.getByRole("dialog"); expect(searchButton.parentElement).toHaveTextContent("Model"); expect(searchButton).toHaveClass("mr-3", "h-6", "w-6"); - expect(picker).toHaveClass("w-[26.25rem]"); + expect(picker).toHaveClass("w-[28.25rem]"); expect(within(picker).getByText("Claude Sonnet 4")).toBeInTheDocument(); expect(within(picker).queryByText("GPT-4o mini")).not.toBeInTheDocument(); expect( @@ -940,7 +942,7 @@ describe("AgentModelPicker", () => { expect( within(picker).queryByText("gpt-4o-mini-2024-07-18"), ).not.toBeInTheDocument(); - expect(picker).toHaveClass("w-[26.25rem]"); + expect(picker).toHaveClass("w-[28.25rem]"); if (modelViewport) { modelViewport.scrollTop = 120; @@ -967,7 +969,7 @@ describe("AgentModelPicker", () => { ).not.toBeInTheDocument(); await user.click( - within(picker).getByRole("button", { name: /GPT-4o mini/ }), + within(picker).getByRole("button", { name: /^GPT-4o mini$/ }), ); // The selection is recorded as recently used, so it joins the compact @@ -1411,11 +1413,11 @@ describe("AgentModelPicker", () => { await openPicker(user); const content = document.querySelector('[data-slot="popover-content"]'); - expect(content).toHaveClass("w-[26.25rem]"); + expect(content).toHaveClass("w-[28.25rem]"); await user.click(screen.getByRole("button", { name: /switch agent/i })); - expect(content).toHaveClass("w-[37.25rem]"); + expect(content).toHaveClass("w-[39.25rem]"); }); it("hides the switch-agent button when the only agent is ready", async () => { @@ -1744,3 +1746,119 @@ describe("AgentModelPicker", () => { }); }); }); + +describe("AgentModelPicker starred models", () => { + beforeEach(() => { + localStorage.clear(); + __resetStarredModelsCacheForTests(); + }); + + afterEach(() => { + localStorage.clear(); + __resetStarredModelsCacheForTests(); + }); + + const models = [ + { id: "preferred", name: "Preferred", recommended: true }, + { id: "also-preferred", name: "Also Preferred", recommended: true }, + { id: "other", name: "Other" }, + { id: "another", name: "Another" }, + ]; + + it("shows star actions on the preferred shortlist", async () => { + const user = userEvent.setup(); + render( + , + ); + + await user.click( + screen.getByRole("button", { name: /choose agent and model/i }), + ); + const picker = screen.getByRole("dialog"); + + expect( + within(picker).getByRole("button", { name: "Star Preferred" }), + ).toBeInTheDocument(); + expect( + within(picker).getByRole("button", { name: "Star Also Preferred" }), + ).toBeInTheDocument(); + expect(within(picker).queryByText("Other")).not.toBeInTheDocument(); + }); + + it("always shows a non-recommended star above the preferred shortlist", async () => { + localStorage.setItem( + STARRED_MODELS_KEY, + JSON.stringify([modelStarKey("goose", "other")]), + ); + __resetStarredModelsCacheForTests(); + const user = userEvent.setup(); + render( + , + ); + + await user.click( + screen.getByRole("button", { name: /choose agent and model/i }), + ); + const starredRow = document.querySelector( + `[data-model-key='${modelStarKey("goose", "other")}']`, + ); + const divider = screen.getByTestId("starred-models-divider"); + const preferredRow = document.querySelector( + `[data-model-key='${modelStarKey("goose", "preferred")}']`, + ); + + expect(starredRow).toBeInTheDocument(); + expect(preferredRow).toBeInTheDocument(); + expect(starredRow?.compareDocumentPosition(divider)).toBe( + Node.DOCUMENT_POSITION_FOLLOWING, + ); + expect(divider.compareDocumentPosition(preferredRow as Element)).toBe( + Node.DOCUMENT_POSITION_FOLLOWING, + ); + expect(screen.queryByText("Another")).not.toBeInTheDocument(); + }); + + it("groups stars in View more without selecting the model", async () => { + const user = userEvent.setup(); + const onModelChange = vi.fn(); + render( + , + ); + + await user.click( + screen.getByRole("button", { name: /choose agent and model/i }), + ); + const picker = screen.getByRole("dialog"); + await user.click(within(picker).getByRole("button", { name: "View more" })); + await user.click( + within(picker).getByRole("button", { name: "Star Other" }), + ); + + expect(onModelChange).not.toHaveBeenCalled(); + expect(screen.getByTestId("starred-models-divider")).toBeInTheDocument(); + }); +}); diff --git a/src/features/design-system/ui/designSystemSections.ts b/src/features/design-system/ui/designSystemSections.ts index 1a1729e85..d3aaeced5 100644 --- a/src/features/design-system/ui/designSystemSections.ts +++ b/src/features/design-system/ui/designSystemSections.ts @@ -124,6 +124,7 @@ export const DESIGN_SYSTEM_COMPONENT_SECTIONS: Array<{ { id: "component-progress", label: "Progress" }, { id: "component-radio-group", label: "Radio Group" }, { id: "component-scroll-area", label: "Scroll Area" }, + { id: "component-separator", label: "Separator" }, { id: "component-searchable-select", label: "Searchable Select" }, { id: "component-search-bar", label: "Search Bar" }, { @@ -168,7 +169,6 @@ export const DESIGN_SYSTEM_UNUSED_COMPONENT_SECTIONS: Array<{ { id: "component-page-columns", label: "Page Columns" }, { id: "component-pagination", label: "Pagination" }, { id: "component-resizable-handle", label: "Resizable Handle" }, - { id: "component-separator", label: "Separator" }, { id: "component-sidebar", label: "Sidebar" }, { id: "component-table", label: "Table" }, { id: "component-toggle", label: "Toggle" }, diff --git a/src/shared/i18n/locales/en/chat.json b/src/shared/i18n/locales/en/chat.json index a1a783d40..96fd88f0a 100644 --- a/src/shared/i18n/locales/en/chat.json +++ b/src/shared/i18n/locales/en/chat.json @@ -636,7 +636,9 @@ "agent-speaking": "Agent is speaking…", "error": "Voice conversation error: {{error}}" } - } + }, + "starModel": "Star {{model}}", + "unstarModel": "Unstar {{model}}" }, "tools": { "content": "Content", diff --git a/src/shared/i18n/locales/es/chat.json b/src/shared/i18n/locales/es/chat.json index 720104f25..83bd07df3 100644 --- a/src/shared/i18n/locales/es/chat.json +++ b/src/shared/i18n/locales/es/chat.json @@ -621,7 +621,9 @@ "agent-speaking": "El agente está hablando…", "error": "Error de conversación de voz: {{error}}" } - } + }, + "starModel": "Destacar {{model}}", + "unstarModel": "Quitar destaque de {{model}}" }, "tools": { "content": "Contenido", From 0ed3806dde75aaa252c1a011ec3764c00f13bcbe Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Thu, 3 Sep 2026 15:56:53 -0400 Subject: [PATCH 02/24] fix(chat): keep starred models visible in the compact picker list Co-authored-by: Goose --- src/features/chat/ui/AgentModelPickerLists.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/features/chat/ui/AgentModelPickerLists.tsx b/src/features/chat/ui/AgentModelPickerLists.tsx index 15a129d69..0840f533d 100644 --- a/src/features/chat/ui/AgentModelPickerLists.tsx +++ b/src/features/chat/ui/AgentModelPickerLists.tsx @@ -246,7 +246,7 @@ export const RecommendedModelList = forwardRef< modelMatchesSelection(model, currentModelId, currentModelProviderId), ); if (current) { - return [current, ...shortlist]; + return [...starred, current, ...shortlist]; } } const unstarredFallback = models.filter( From 8811486e936b9e03906fe426fe8cc7c74e86fa26 Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Thu, 3 Sep 2026 15:57:36 -0400 Subject: [PATCH 03/24] fix(chat): fit picker within min window width; hover-reveal stars with softer fill Co-authored-by: Goose --- src/features/chat/ui/AgentModelPicker.tsx | 4 +++- src/features/chat/ui/AgentModelPickerLists.tsx | 8 ++++---- src/features/chat/ui/__tests__/AgentModelPicker.test.tsx | 8 ++++---- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/features/chat/ui/AgentModelPicker.tsx b/src/features/chat/ui/AgentModelPicker.tsx index 313c197c0..efc78e178 100644 --- a/src/features/chat/ui/AgentModelPicker.tsx +++ b/src/features/chat/ui/AgentModelPicker.tsx @@ -500,7 +500,9 @@ export function AgentModelPicker({ // gated single-column layout has no dead vertical space below the // model list. "flex max-h-[min(24rem,50vh)] flex-col overflow-hidden p-1 transition-[width] duration-[240ms] ease-[cubic-bezier(0.2,0,0,1)]", - isWidePicker ? "w-[39.25rem]" : "w-[28.25rem]", + isWidePicker + ? "w-[min(39.25rem,calc(100vw-1.5rem))]" + : "w-[min(28.25rem,calc(100vw-1.5rem))]", )} onInteractOutside={(event) => { classifyOutsideInteraction(event.target); diff --git a/src/features/chat/ui/AgentModelPickerLists.tsx b/src/features/chat/ui/AgentModelPickerLists.tsx index 0840f533d..f78b91790 100644 --- a/src/features/chat/ui/AgentModelPickerLists.tsx +++ b/src/features/chat/ui/AgentModelPickerLists.tsx @@ -456,7 +456,7 @@ export const RecommendedModelList = forwardRef< return (
@@ -488,7 +488,7 @@ export const RecommendedModelList = forwardRef<
diff --git a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx index c7e375a1d..303f57342 100644 --- a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx +++ b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx @@ -880,7 +880,7 @@ describe("AgentModelPicker", () => { const picker = screen.getByRole("dialog"); expect(searchButton.parentElement).toHaveTextContent("Model"); expect(searchButton).toHaveClass("mr-3", "h-6", "w-6"); - expect(picker).toHaveClass("w-[28.25rem]"); + expect(picker).toHaveClass("w-[min(28.25rem,calc(100vw-1.5rem))]"); expect(within(picker).getByText("Claude Sonnet 4")).toBeInTheDocument(); expect(within(picker).queryByText("GPT-4o mini")).not.toBeInTheDocument(); expect( @@ -942,7 +942,7 @@ describe("AgentModelPicker", () => { expect( within(picker).queryByText("gpt-4o-mini-2024-07-18"), ).not.toBeInTheDocument(); - expect(picker).toHaveClass("w-[28.25rem]"); + expect(picker).toHaveClass("w-[min(28.25rem,calc(100vw-1.5rem))]"); if (modelViewport) { modelViewport.scrollTop = 120; @@ -1413,11 +1413,11 @@ describe("AgentModelPicker", () => { await openPicker(user); const content = document.querySelector('[data-slot="popover-content"]'); - expect(content).toHaveClass("w-[28.25rem]"); + expect(content).toHaveClass("w-[min(28.25rem,calc(100vw-1.5rem))]"); await user.click(screen.getByRole("button", { name: /switch agent/i })); - expect(content).toHaveClass("w-[39.25rem]"); + expect(content).toHaveClass("w-[min(39.25rem,calc(100vw-1.5rem))]"); }); it("hides the switch-agent button when the only agent is ready", async () => { From d8b07c59b2a95e6c4e2fa0fee7035f01caf8e3dd Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Thu, 3 Sep 2026 17:07:25 -0400 Subject: [PATCH 04/24] fix(chat): render star action through shared Button with semantic selected state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the hand-rolled star + {starred ? : } +
{showStarredDivider ? ( { expect(onModelChange).not.toHaveBeenCalled(); expect(screen.getByTestId("starred-models-divider")).toBeInTheDocument(); }); + + it("renders star actions through the shared Button contract with a ≥3:1 idle treatment", async () => { + localStorage.setItem( + STARRED_MODELS_KEY, + JSON.stringify([modelStarKey("goose", "other")]), + ); + __resetStarredModelsCacheForTests(); + const user = userEvent.setup(); + render( + , + ); + + await user.click( + screen.getByRole("button", { name: /choose agent and model/i }), + ); + const picker = screen.getByRole("dialog"); + + // Unstarred rows idle on the ghost icon contract's muted-foreground; the + // pairing against the popover surface is enforced in globals.test.ts. + const idleStar = within(picker).getByRole("button", { + name: "Star Preferred", + }); + expect(idleStar).toHaveAttribute("data-slot", "button"); + expect(idleStar).toHaveAttribute("aria-pressed", "false"); + expect(idleStar).toHaveClass("text-muted-foreground"); + expect(idleStar).not.toHaveClass("text-foreground/80"); + + const starredToggle = within(picker).getByRole("button", { + name: "Unstar Other", + }); + expect(starredToggle).toHaveAttribute("data-slot", "button"); + expect(starredToggle).toHaveAttribute("aria-pressed", "true"); + expect(starredToggle).toHaveClass("text-foreground/80"); + expect(starredToggle).not.toHaveClass("text-muted-foreground"); + }); }); diff --git a/src/features/design-system/generated/componentManifest.ts b/src/features/design-system/generated/componentManifest.ts index 9943ca385..bce486648 100644 --- a/src/features/design-system/generated/componentManifest.ts +++ b/src/features/design-system/generated/componentManifest.ts @@ -595,12 +595,13 @@ export const designSystemComponentManifest = [ ], destructive: ["true", "false"], flush: ["true", "false"], + selected: ["true", "false"], }, defaultVariants: { variant: "primary", size: "default", }, - compoundVariantCount: 11, + compoundVariantCount: 12, tokenClasses: [ "active:text-foreground", "aria-expanded:text-foreground", @@ -625,6 +626,7 @@ export const designSystemComponentManifest = [ "text-accent-foreground", "text-destructive", "text-destructive-foreground", + "text-foreground/80", "text-muted-foreground", "text-primary", "text-primary-foreground", @@ -680,6 +682,7 @@ export const designSystemComponentManifest = [ "text-accent-foreground", "text-destructive", "text-destructive-foreground", + "text-foreground/80", "text-muted-foreground", "text-primary", "text-primary-foreground", @@ -710,6 +713,7 @@ export const designSystemComponentManifest = [ "text-accent-foreground", "text-destructive", "text-destructive-foreground", + "text-foreground/80", "text-muted-foreground", "text-primary", "text-primary-foreground", @@ -766,6 +770,7 @@ export const designSystemComponentManifest = [ "text-accent-foreground", "text-destructive", "text-destructive-foreground", + "text-foreground/80", "text-muted-foreground", "text-primary", "text-primary-foreground", diff --git a/src/shared/styles/globals.test.ts b/src/shared/styles/globals.test.ts index f7e888cf5..20dc6bc04 100644 --- a/src/shared/styles/globals.test.ts +++ b/src/shared/styles/globals.test.ts @@ -93,3 +93,82 @@ describe("background token", () => { ); }); }); + +type TokenDeclarations = Map; + +function declarationsMap(selector: string): TokenDeclarations { + const map: TokenDeclarations = new Map(); + for (const match of declarationsFor(selector).matchAll( + /^\s*(--[\w-]+):\s*([^;]+);/gm, + )) { + map.set(match[1], match[2].trim()); + } + return map; +} + +/** Resolve a custom property to a literal color, following var() chains. */ +function resolveToken( + token: string, + theme: TokenDeclarations, + palette: TokenDeclarations, +): string { + let value: string = theme.get(token) ?? palette.get(token) ?? ""; + if (value === "") { + throw new Error(`Missing ${token}`); + } + const seen = new Set(); + for (;;) { + const ref = value.match(/^var\((--[\w-]+)(?:,\s*([^)]+))?\)$/); + if (!ref) { + return value; + } + if (seen.has(ref[1])) { + throw new Error(`Circular var() reference at ${ref[1]}`); + } + seen.add(ref[1]); + const next: string = + theme.get(ref[1]) ?? palette.get(ref[1]) ?? ref[2]?.trim() ?? ""; + if (next === "") { + throw new Error(`Unresolved var() reference ${ref[1]} in ${token}`); + } + value = next; + } +} + +function srgbChannelToLinear(channel: number): number { + return channel <= 0.04045 + ? channel / 12.92 + : ((channel + 0.055) / 1.055) ** 2.4; +} + +/** WCAG 2.x relative luminance of a #rrggbb color. */ +function relativeLuminance(hex: string): number { + const digits = hex.replace(/^#/, ""); + if (!/^[0-9a-fA-F]{6}$/.test(digits)) { + throw new Error(`Unsupported color for contrast math: ${hex}`); + } + const [red, green, blue] = [0, 2, 4].map((offset) => + srgbChannelToLinear(parseInt(digits.slice(offset, offset + 2), 16) / 255), + ); + return 0.2126 * red + 0.7152 * green + 0.0722 * blue; +} + +function contrastRatio(foreground: string, background: string): number { + const [lighter, darker] = [ + relativeLuminance(foreground), + relativeLuminance(background), + ].sort((left, right) => right - left); + return (lighter + 0.05) / (darker + 0.05); +} + +describe("muted-foreground on popover", () => { + it("clears 3:1 non-text contrast in both themes for icon-only controls (model picker star)", () => { + const palette = declarationsMap("@theme {"); + for (const selector of [":root {", '[data-theme="dark"],']) { + const theme = declarationsMap(selector); + const foreground = resolveToken("--muted-foreground", theme, palette); + const background = resolveToken("--popover", theme, palette); + expect(contrastRatio(foreground, background)).toBeGreaterThanOrEqual(3); + } + }); +}); diff --git a/src/shared/ui/button.tsx b/src/shared/ui/button.tsx index f24c27dec..171de69c0 100644 --- a/src/shared/ui/button.tsx +++ b/src/shared/ui/button.tsx @@ -54,6 +54,10 @@ const buttonVariants = cva( true: "", false: "", }, + selected: { + true: "", + false: "", + }, }, compoundVariants: [ { @@ -120,12 +124,18 @@ const buttonVariants = cva( className: "bg-transparent text-muted-foreground hover:bg-transparent hover:text-foreground active:bg-transparent data-[state=open]:text-foreground aria-expanded:text-foreground", }, + { + variant: "ghost", + selected: true, + className: "text-foreground/80", + }, ], defaultVariants: { variant: "primary", size: "default", destructive: false, flush: false, + selected: false, }, }, ); @@ -279,7 +289,7 @@ export function isButtonDestructiveEmphasis( } export type ButtonProps = React.ButtonHTMLAttributes & - Omit & { + Omit & { /** * Danger intent. Recolors the emphasis recipe with destructive tokens: * primary = red fill, outline = red border + red text, subtle = red @@ -296,6 +306,15 @@ export type ButtonProps = React.ButtonHTMLAttributes & * on ghost; other variants ignore it (dev builds warn). */ flush?: boolean; + /** + * On/pressed state for toggle controls that lean on Button as their + * chrome (icon toggles like the model picker star). Ghost buttons rest + * the label or icon at foreground/80 — softer than idle content yet + * well above the 3:1 non-text contrast bar — and keep the ghost hover + * at full foreground. Only meaningful on ghost; other variants ignore + * it (dev builds warn). + */ + selected?: boolean; asChild?: boolean; leftIcon?: React.ReactNode; rightIcon?: React.ReactNode; @@ -322,6 +341,7 @@ const Button = React.forwardRef( size, destructive, flush, + selected, asChild = false, leftIcon, rightIcon, @@ -361,6 +381,12 @@ const Button = React.forwardRef( `Button: the flush flag is only supported on the ghost variant; it is ignored on variant="${resolvedEmphasis}".`, ); } + const resolvedSelected = Boolean(selected && resolvedEmphasis === "ghost"); + if (import.meta.env.DEV && selected && !resolvedSelected) { + console.warn( + `Button: the selected flag is only supported on the ghost variant; it is ignored on variant="${resolvedEmphasis}".`, + ); + } const Comp = asChild ? Slot : "button"; const renderedChildren = asChild ? children @@ -517,6 +543,7 @@ const Button = React.forwardRef( props: { destructive: resolvedDestructive, flush: resolvedFlush, + selected: resolvedSelected, asChild, disabled: resolvedDisabled, leftIcon: Boolean(resolvedLeftIcon), @@ -543,6 +570,7 @@ const Button = React.forwardRef( size, destructive: resolvedDestructive, flush: resolvedFlush, + selected: resolvedSelected, className, }), asChild && resolvedDisabled && "pointer-events-none", From af9ffaa15ea63bcbd91ec0bfff753c1696a0e623 Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Thu, 3 Sep 2026 17:19:29 -0400 Subject: [PATCH 05/24] fix(chat): store each starred model as its own localStorage entry toggleModelStar previously rewrote one aggregate key via read-modify-write, so two windows toggling different models at the same time could drop each other's stars (last write wins). Each star now lives under its own key (goose:starredModels:v1:entry:), so a toggle touches exactly one key and cannot clobber others. The same-model instant-interleave race remains; per-model state stays consistent either way. The storage-event listener in useStarredModels now matches the entry prefix (and clears) instead of one key. A one-shot, idempotent migration folds the legacy aggregate array into per-key entries on first read or toggle, so existing dev-build stars survive. Co-authored-by: Goose --- src/features/chat/hooks/useStarredModels.ts | 10 +- src/features/chat/lib/starredModels.ts | 110 ++++++++++++----- .../ui/__tests__/AgentModelPicker.test.tsx | 113 ++++++++++++++++-- 3 files changed, 195 insertions(+), 38 deletions(-) diff --git a/src/features/chat/hooks/useStarredModels.ts b/src/features/chat/hooks/useStarredModels.ts index 843a93628..f7ad84964 100644 --- a/src/features/chat/hooks/useStarredModels.ts +++ b/src/features/chat/hooks/useStarredModels.ts @@ -2,8 +2,8 @@ import { useCallback, useSyncExternalStore } from "react"; import { getStarredModelKeys, modelStarKey, + STARRED_MODELS_ENTRY_PREFIX, STARRED_MODELS_EVENT, - STARRED_MODELS_KEY, toggleModelStar, } from "../lib/starredModels"; @@ -28,7 +28,13 @@ function subscribe(callback: () => void): () => void { callback(); }; const handleStorage = (event: StorageEvent) => { - if (event.key === null || event.key === STARRED_MODELS_KEY) { + // Star entries live under per-key storage, so any entry write or removal + // in another window changes the set. `key === null` covers localStorage + // clears. + if ( + event.key === null || + event.key.startsWith(STARRED_MODELS_ENTRY_PREFIX) + ) { handleChange(); } }; diff --git a/src/features/chat/lib/starredModels.ts b/src/features/chat/lib/starredModels.ts index c49fcdf86..5c44f5a97 100644 --- a/src/features/chat/lib/starredModels.ts +++ b/src/features/chat/lib/starredModels.ts @@ -1,4 +1,6 @@ -const STARRED_MODELS_STORAGE_KEY = "goose:starredModels:v1"; +export const STARRED_MODELS_ENTRY_PREFIX = "goose:starredModels:v1:entry:"; +export const LEGACY_STARRED_MODELS_STORAGE_KEY = "goose:starredModels:v1"; +const STARRED_MODELS_ENTRY_VALUE = "1"; const STARRED_MODELS_CHANGED_EVENT = "goose:starred-models-changed"; type StarredModelSet = Set; @@ -7,44 +9,93 @@ export function modelStarKey(scopeId: string, modelId: string): string { return JSON.stringify([scopeId, modelId]); } +/** localStorage key of the single entry that records one starred model. */ +export function starredModelStorageKey(starKey: string): string { + return STARRED_MODELS_ENTRY_PREFIX + encodeURIComponent(starKey); +} + +/** + * One-shot migration from the pre-#287 aggregate format (one array under a + * single key). Idempotent and safe to race across windows: entry writes are + * identical values, and removing the legacy key twice is a no-op. + */ +function migrateLegacyStarredModels(): void { + const legacy = window.localStorage.getItem(LEGACY_STARRED_MODELS_STORAGE_KEY); + if (legacy === null) { + return; + } + + window.localStorage.removeItem(LEGACY_STARRED_MODELS_STORAGE_KEY); + + try { + const parsed: unknown = JSON.parse(legacy); + if (!Array.isArray(parsed)) { + return; + } + for (const item of parsed) { + if (typeof item === "string") { + window.localStorage.setItem( + starredModelStorageKey(item), + STARRED_MODELS_ENTRY_VALUE, + ); + } + } + } catch { + // Unreadable legacy value; the key has already been removed. + } +} + function readStarredModels(): StarredModelSet { if (typeof window === "undefined") { return new Set(); } try { - const stored = window.localStorage.getItem(STARRED_MODELS_STORAGE_KEY); - if (!stored) { - return new Set(); - } - - const parsed = JSON.parse(stored); - if (!Array.isArray(parsed)) { - return new Set(); + migrateLegacyStarredModels(); + const storage = window.localStorage; + const starred = new Set(); + for (let i = 0; i < storage.length; i += 1) { + const storageKey = storage.key(i); + if (!storageKey?.startsWith(STARRED_MODELS_ENTRY_PREFIX)) { + continue; + } + try { + starred.add( + decodeURIComponent( + storageKey.slice(STARRED_MODELS_ENTRY_PREFIX.length), + ), + ); + } catch { + // Skip a malformed entry rather than dropping every star. + } } - - return new Set(parsed.filter((item) => typeof item === "string")); + return starred; } catch { return new Set(); } } -function persistStarredModels(models: StarredModelSet): void { +/** + * Write or clear exactly one star entry. Touching a single key (instead of + * rewriting an aggregate array) removes the cross-window read-modify-write + * race where concurrent toggles from different windows could drop each + * other's stars. Note that two windows toggling the same model at the same + * instant can still interleave; per-model state stays consistent either way. + */ +function persistStarEntry(starKey: string, starred: boolean): void { if (typeof window === "undefined") { return; } try { - if (models.size === 0) { - window.localStorage.removeItem(STARRED_MODELS_STORAGE_KEY); + const storageKey = starredModelStorageKey(starKey); + if (starred) { + window.localStorage.setItem(storageKey, STARRED_MODELS_ENTRY_VALUE); } else { - window.localStorage.setItem( - STARRED_MODELS_STORAGE_KEY, - JSON.stringify([...models]), - ); + window.localStorage.removeItem(storageKey); } } catch { - // localStorage may be unavailable. + // localStorage may be unavailable or over quota. } window.dispatchEvent(new CustomEvent(STARRED_MODELS_CHANGED_EVENT)); @@ -55,15 +106,20 @@ export function getStarredModelKeys(): StarredModelSet { } export function toggleModelStar(scopeId: string, modelId: string): void { - const next = readStarredModels(); - const key = modelStarKey(scopeId, modelId); - if (next.has(key)) { - next.delete(key); - } else { - next.add(key); + if (typeof window === "undefined") { + return; + } + + const starKey = modelStarKey(scopeId, modelId); + + try { + migrateLegacyStarredModels(); + const starred = + window.localStorage.getItem(starredModelStorageKey(starKey)) !== null; + persistStarEntry(starKey, !starred); + } catch { + // localStorage may be unavailable; leave stored state untouched. } - persistStarredModels(next); } export const STARRED_MODELS_EVENT = STARRED_MODELS_CHANGED_EVENT; -export const STARRED_MODELS_KEY = STARRED_MODELS_STORAGE_KEY; diff --git a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx index 6a30ec3bf..7ecd740cd 100644 --- a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx +++ b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx @@ -3,7 +3,11 @@ import { render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { __resetStarredModelsCacheForTests } from "../../hooks/useStarredModels"; -import { modelStarKey, STARRED_MODELS_KEY } from "../../lib/starredModels"; +import { + LEGACY_STARRED_MODELS_STORAGE_KEY, + modelStarKey, + starredModelStorageKey, +} from "../../lib/starredModels"; import { AgentModelPicker } from "../AgentModelPicker"; import { getModelRecencyMap, @@ -1765,6 +1769,13 @@ describe("AgentModelPicker starred models", () => { { id: "another", name: "Another" }, ]; + const seedStar = (scopeId: string, modelId: string) => { + localStorage.setItem( + starredModelStorageKey(modelStarKey(scopeId, modelId)), + "1", + ); + }; + it("shows star actions on the preferred shortlist", async () => { const user = userEvent.setup(); render( @@ -1794,10 +1805,7 @@ describe("AgentModelPicker starred models", () => { }); it("always shows a non-recommended star above the preferred shortlist", async () => { - localStorage.setItem( - STARRED_MODELS_KEY, - JSON.stringify([modelStarKey("goose", "other")]), - ); + seedStar("goose", "other"); __resetStarredModelsCacheForTests(); const user = userEvent.setup(); render( @@ -1863,10 +1871,7 @@ describe("AgentModelPicker starred models", () => { }); it("renders star actions through the shared Button contract with a ≥3:1 idle treatment", async () => { - localStorage.setItem( - STARRED_MODELS_KEY, - JSON.stringify([modelStarKey("goose", "other")]), - ); + seedStar("goose", "other"); __resetStarredModelsCacheForTests(); const user = userEvent.setup(); render( @@ -1904,4 +1909,94 @@ describe("AgentModelPicker starred models", () => { expect(starredToggle).toHaveClass("text-foreground/80"); expect(starredToggle).not.toHaveClass("text-muted-foreground"); }); + + it("migrates the legacy aggregate entry into per-key entries", async () => { + localStorage.setItem( + LEGACY_STARRED_MODELS_STORAGE_KEY, + JSON.stringify([modelStarKey("goose", "other"), 42, null]), + ); + __resetStarredModelsCacheForTests(); + const user = userEvent.setup(); + render( + , + ); + + await user.click( + screen.getByRole("button", { name: /choose agent and model/i }), + ); + + expect( + document.querySelector( + `[data-model-key='${modelStarKey("goose", "other")}']`, + ), + ).toBeInTheDocument(); + expect(localStorage.getItem(LEGACY_STARRED_MODELS_STORAGE_KEY)).toBeNull(); + expect( + localStorage.getItem( + starredModelStorageKey(modelStarKey("goose", "other")), + ), + ).toBe("1"); + }); + + it("stores each star as its own entry so one toggle cannot drop another", async () => { + seedStar("goose", "other"); + __resetStarredModelsCacheForTests(); + const user = userEvent.setup(); + render( + , + ); + + await user.click( + screen.getByRole("button", { name: /choose agent and model/i }), + ); + const picker = screen.getByRole("dialog"); + await user.click(within(picker).getByRole("button", { name: "View more" })); + await user.click( + within(picker).getByRole("button", { name: "Star Another" }), + ); + + // Starring one model must leave every other star entry untouched; an + // aggregate rewrite from a stale snapshot would drop "other" here. + expect( + localStorage.getItem( + starredModelStorageKey(modelStarKey("goose", "other")), + ), + ).toBe("1"); + expect( + localStorage.getItem( + starredModelStorageKey(modelStarKey("goose", "another")), + ), + ).toBe("1"); + + await user.click( + within(picker).getByRole("button", { name: "Unstar Other" }), + ); + + expect( + localStorage.getItem( + starredModelStorageKey(modelStarKey("goose", "other")), + ), + ).toBeNull(); + expect( + localStorage.getItem( + starredModelStorageKey(modelStarKey("goose", "another")), + ), + ).toBe("1"); + }); }); From 34d27b32f4139054aa9e104338a72abbbff12a2a Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Thu, 3 Sep 2026 17:27:40 -0400 Subject: [PATCH 06/24] fix(chat): toast when starred-model writes fail instead of swallowing persistStarEntry caught localStorage failures (quota exceeded, unavailable storage) silently, so a star toggle would appear to do nothing with no explanation. Failures on write and remove now surface a toast.error with the new chat:notifications.starredModelsPersistError string (en + es). The read path in toggleModelStar reports the same failure when storage is entirely unavailable. Tests stub Storage.prototype setItem/removeItem to throw and assert one toast fires, the entry is untouched, and the toggle state bounces back to the stored truth. Co-authored-by: Goose --- src/features/chat/lib/starredModels.ts | 11 +- .../ui/__tests__/AgentModelPicker.test.tsx | 109 ++++++++++++++++++ src/shared/i18n/locales/en/chat.json | 1 + src/shared/i18n/locales/es/chat.json | 1 + 4 files changed, 120 insertions(+), 2 deletions(-) diff --git a/src/features/chat/lib/starredModels.ts b/src/features/chat/lib/starredModels.ts index 5c44f5a97..52a540e5e 100644 --- a/src/features/chat/lib/starredModels.ts +++ b/src/features/chat/lib/starredModels.ts @@ -1,3 +1,6 @@ +import { toast } from "sonner"; +import { i18n } from "@/shared/i18n"; + export const STARRED_MODELS_ENTRY_PREFIX = "goose:starredModels:v1:entry:"; export const LEGACY_STARRED_MODELS_STORAGE_KEY = "goose:starredModels:v1"; const STARRED_MODELS_ENTRY_VALUE = "1"; @@ -95,7 +98,9 @@ function persistStarEntry(starKey: string, starred: boolean): void { window.localStorage.removeItem(storageKey); } } catch { - // localStorage may be unavailable or over quota. + // The write did not land (storage unavailable or over quota). Tell the + // user instead of letting the toggle silently bounce back. + toast.error(i18n.t("chat:notifications.starredModelsPersistError")); } window.dispatchEvent(new CustomEvent(STARRED_MODELS_CHANGED_EVENT)); @@ -118,7 +123,9 @@ export function toggleModelStar(scopeId: string, modelId: string): void { window.localStorage.getItem(starredModelStorageKey(starKey)) !== null; persistStarEntry(starKey, !starred); } catch { - // localStorage may be unavailable; leave stored state untouched. + // Storage is unavailable, so the toggle cannot be applied at all. The + // write path reports its own failures; report this one too. + toast.error(i18n.t("chat:notifications.starredModelsPersistError")); } } diff --git a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx index 7ecd740cd..c0cd47fe7 100644 --- a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx +++ b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx @@ -16,6 +16,18 @@ import { recordModelSelection, } from "../../lib/modelRecency"; import { OPEN_SETTINGS_EVENT } from "@/features/settings/lib/settingsEvents"; +import { toast } from "sonner"; + +vi.mock("sonner", () => ({ + toast: { + error: vi.fn(), + success: vi.fn(), + info: vi.fn(), + warning: vi.fn(), + message: vi.fn(), + dismiss: vi.fn(), + }, +})); class ResizeObserverStub { observe() {} @@ -1755,6 +1767,7 @@ describe("AgentModelPicker starred models", () => { beforeEach(() => { localStorage.clear(); __resetStarredModelsCacheForTests(); + vi.mocked(toast.error).mockClear(); }); afterEach(() => { @@ -1999,4 +2012,100 @@ describe("AgentModelPicker starred models", () => { ), ).toBe("1"); }); + + it("surfaces a persist failure when starring cannot be saved", async () => { + const setItemSpy = vi + .spyOn(Storage.prototype, "setItem") + .mockImplementation(() => { + throw new DOMException("quota exceeded", "QuotaExceededError"); + }); + const user = userEvent.setup(); + render( + , + ); + + try { + await user.click( + screen.getByRole("button", { name: /choose agent and model/i }), + ); + const picker = screen.getByRole("dialog"); + await user.click( + within(picker).getByRole("button", { name: "View more" }), + ); + await user.click( + within(picker).getByRole("button", { name: "Star Another" }), + ); + + expect(vi.mocked(toast.error)).toHaveBeenCalledTimes(1); + expect(vi.mocked(toast.error)).toHaveBeenCalledWith( + expect.stringMatching(/starred/i), + ); + expect( + localStorage.getItem( + starredModelStorageKey(modelStarKey("goose", "another")), + ), + ).toBeNull(); + // The optimistic toggle must not stick when the write failed. + expect( + within(picker).getByRole("button", { name: "Star Another" }), + ).toHaveAttribute("aria-pressed", "false"); + } finally { + setItemSpy.mockRestore(); + } + }); + + it("surfaces a persist failure when unstarring cannot be saved", async () => { + seedStar("goose", "other"); + __resetStarredModelsCacheForTests(); + const removeItemSpy = vi + .spyOn(Storage.prototype, "removeItem") + .mockImplementation(() => { + throw new DOMException("quota exceeded", "QuotaExceededError"); + }); + const user = userEvent.setup(); + render( + , + ); + + try { + await user.click( + screen.getByRole("button", { name: /choose agent and model/i }), + ); + const picker = screen.getByRole("dialog"); + await user.click( + within(picker).getByRole("button", { name: "Unstar Other" }), + ); + + expect(vi.mocked(toast.error)).toHaveBeenCalledTimes(1); + expect(vi.mocked(toast.error)).toHaveBeenCalledWith( + expect.stringMatching(/starred/i), + ); + expect( + localStorage.getItem( + starredModelStorageKey(modelStarKey("goose", "other")), + ), + ).toBe("1"); + expect( + within(picker).getByRole("button", { name: "Unstar Other" }), + ).toHaveAttribute("aria-pressed", "true"); + } finally { + removeItemSpy.mockRestore(); + } + }); }); diff --git a/src/shared/i18n/locales/en/chat.json b/src/shared/i18n/locales/en/chat.json index 96fd88f0a..2cf1f4fa4 100644 --- a/src/shared/i18n/locales/en/chat.json +++ b/src/shared/i18n/locales/en/chat.json @@ -368,6 +368,7 @@ "voiceStopBeforeArchiveError": "Couldn't stop voice, so the chat wasn't archived", "gitInspectionError": "Couldn't inspect the worktrees or branches. The chat wasn't archived.", "gitCleanupError": "Chat archived, but Git cleanup couldn't finish", + "starredModelsPersistError": "Couldn't update your starred models. Device storage may be full or unavailable.", "moveError": "Failed to move chat", "renameError": "Failed to rename chat" }, diff --git a/src/shared/i18n/locales/es/chat.json b/src/shared/i18n/locales/es/chat.json index 83bd07df3..5d2ebab5a 100644 --- a/src/shared/i18n/locales/es/chat.json +++ b/src/shared/i18n/locales/es/chat.json @@ -367,6 +367,7 @@ "voiceStopBeforeArchiveError": "No se pudo detener la conversación de voz, así que el chat no se archivó", "gitInspectionError": "No se pudieron inspeccionar los worktrees o las ramas. El chat no se archivó.", "gitCleanupError": "El chat se archivó, pero no se pudo completar la limpieza de Git", + "starredModelsPersistError": "No se pudieron actualizar tus modelos favoritos. El almacenamiento del dispositivo puede estar lleno o no disponible.", "moveError": "No se pudo mover el chat", "renameError": "No se pudo cambiar el nombre del chat" }, From b112d294c770f47bb35add314df42e2c11faba81 Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Thu, 3 Sep 2026 17:36:34 -0400 Subject: [PATCH 07/24] fix(chat): hide starred models the catalog no longer serves A starred model a provider stopped serving kept rendering as starred through the synthesized row for the current selection: rows otherwise come only from the available models, so dead stars were already invisible everywhere else. RecommendedModelList now takes the raw catalog (catalogModels) and honors starred state only for models present in it. The dropped current model stays visible and selectable but renders unstarred, without a star toggle (a dead model cannot be favorited). Stored entries are kept, so a star returns if the provider serves the model again; hard pruning was rejected because loading/partial-catalog states could wipe stars. Tests: a starred current model dropped by its provider renders unstarred with no toggle or divider (verified red without the fix), and a star for a model absent from the list renders no row while its entry survives. Co-authored-by: Goose --- src/features/chat/ui/AgentModelPicker.tsx | 1 + .../chat/ui/AgentModelPickerLists.tsx | 108 ++++++++++++------ .../ui/__tests__/AgentModelPicker.test.tsx | 85 ++++++++++++++ 3 files changed, 161 insertions(+), 33 deletions(-) diff --git a/src/features/chat/ui/AgentModelPicker.tsx b/src/features/chat/ui/AgentModelPicker.tsx index efc78e178..2069e0be5 100644 --- a/src/features/chat/ui/AgentModelPicker.tsx +++ b/src/features/chat/ui/AgentModelPicker.tsx @@ -703,6 +703,7 @@ export function AgentModelPicker({ key={selectedAgentId} ref={modelListRef} models={displayedModels} + catalogModels={availableModels} currentModelId={currentModelId} currentModelProviderId={currentModelProviderId} selectedAgentId={selectedAgentId} diff --git a/src/features/chat/ui/AgentModelPickerLists.tsx b/src/features/chat/ui/AgentModelPickerLists.tsx index 6841d9dcb..372ad8282 100644 --- a/src/features/chat/ui/AgentModelPickerLists.tsx +++ b/src/features/chat/ui/AgentModelPickerLists.tsx @@ -134,6 +134,14 @@ function sortModels( interface ModelListProps { models: ModelOption[]; + /** + * The authoritative catalog the rows were built from, without any + * synthesized rows for the current selection. Starred state is only + * honored for models present here, so favorited models a provider no + * longer serves stop rendering as starred. Omit to treat every row as + * existing. + */ + catalogModels?: ModelOption[]; currentModelId: string | null; currentModelProviderId: string | null; selectedAgentId: string; @@ -157,6 +165,7 @@ export const RecommendedModelList = forwardRef< >(function RecommendedModelList( { models, + catalogModels, currentModelId, currentModelProviderId, selectedAgentId, @@ -166,7 +175,33 @@ export const RecommendedModelList = forwardRef< }, ref, ) { - const { isStarred, toggleStar, starredKeys } = useStarredModels(); + const { toggleStar, starredKeys } = useStarredModels(); + // Rows include a synthesized entry for the current selection when the + // catalog no longer serves it. Honoring starred state only for catalog + // models keeps a favorited model a provider dropped from rendering as + // starred; the stored entry survives so the star returns if the model does. + const existingModelKeys = useMemo(() => { + if (!catalogModels) { + return null; + } + return new Set( + catalogModels.map((model) => + modelStarKey(model.providerId ?? selectedAgentId, model.id), + ), + ); + }, [catalogModels, selectedAgentId]); + const liveStarredKeys = useMemo(() => { + if (!existingModelKeys) { + return starredKeys; + } + const live = new Set(); + for (const key of starredKeys) { + if (existingModelKeys.has(key)) { + live.add(key); + } + } + return live; + }, [existingModelKeys, starredKeys]); const [searchOpen, setSearchOpen] = useState(false); const [showAll, setShowAll] = useState(false); const [query, setQuery] = useState(""); @@ -191,7 +226,7 @@ export const RecommendedModelList = forwardRef< const recencyMap = useModelRecency(); const recommended = useMemo(() => { const starred = models.filter((model) => - starredKeys.has( + liveStarredKeys.has( modelStarKey(model.providerId ?? selectedAgentId, model.id), ), ); @@ -208,7 +243,7 @@ export const RecommendedModelList = forwardRef< currentModelId, currentModelProviderId, ) && - !starredKeys.has( + !liveStarredKeys.has( modelStarKey( entry.model.providerId ?? selectedAgentId, entry.model.id, @@ -229,7 +264,9 @@ export const RecommendedModelList = forwardRef< .filter( (m) => !recent.some((r) => r.id === m.id && r.providerId === m.providerId) && - !starredKeys.has(modelStarKey(m.providerId ?? selectedAgentId, m.id)), + !liveStarredKeys.has( + modelStarKey(m.providerId ?? selectedAgentId, m.id), + ), ); const shortlist = [...recent, ...rec]; if ( @@ -251,7 +288,7 @@ export const RecommendedModelList = forwardRef< } const unstarredFallback = models.filter( (model) => - !starredKeys.has( + !liveStarredKeys.has( modelStarKey(model.providerId ?? selectedAgentId, model.id), ), ); @@ -265,7 +302,7 @@ export const RecommendedModelList = forwardRef< currentModelProviderId, recencyMap, selectedAgentId, - starredKeys, + liveStarredKeys, ]); useEffect(() => { @@ -313,7 +350,7 @@ export const RecommendedModelList = forwardRef< const unstarred: ModelOption[] = []; for (const model of visibleModels) { const scopeId = model.providerId ?? selectedAgentId; - (starredKeys.has(modelStarKey(scopeId, model.id)) + (liveStarredKeys.has(modelStarKey(scopeId, model.id)) ? starred : unstarred ).push(model); @@ -334,7 +371,7 @@ export const RecommendedModelList = forwardRef< currentModelProviderId, recencyMap, selectedAgentId, - starredKeys, + liveStarredKeys, ]); const sorted = [...grouped.starred, ...grouped.unstarred]; @@ -449,15 +486,18 @@ export const RecommendedModelList = forwardRef< currentModelProviderId, ); const scopeId = model.providerId ?? selectedAgentId; - const starred = isStarred(scopeId, model.id); + const modelKey = modelStarKey(scopeId, model.id); + const starred = liveStarredKeys.has(modelKey); + const existsInCatalog = + !existingModelKeys || existingModelKeys.has(modelKey); const showStarredDivider = index === grouped.starred.length - 1 && grouped.unstarred.length > 0; return ( -
+
) : null} - + {existsInCatalog ? ( + + ) : null}
{showStarredDivider ? ( { removeItemSpy.mockRestore(); } }); + it("renders a starred current model unstarred once its provider drops it", async () => { + seedStar("prov-a", "ghost"); + __resetStarredModelsCacheForTests(); + const user = userEvent.setup(); + render( + , + ); + + await user.click( + screen.getByRole("button", { name: /choose agent and model/i }), + ); + screen.getByRole("dialog"); + + // The dropped selection stays visible so the user can see what is in use... + const ghostRow = document.querySelector( + '[data-model-key=\'["prov-a","ghost"]\']', + ); + expect(ghostRow).toBeInTheDocument(); + // ...but it is no longer a favorite: no star state, no toggle, no divider. + expect(ghostRow).not.toHaveAttribute("data-starred"); + expect( + within(ghostRow as HTMLElement).queryByRole("button", { + name: /star ghost/i, + }), + ).not.toBeInTheDocument(); + expect( + screen.queryByTestId("starred-models-divider"), + ).not.toBeInTheDocument(); + // The stored entry survives so the star returns if the model does. + expect( + localStorage.getItem( + starredModelStorageKey(modelStarKey("prov-a", "ghost")), + ), + ).toBe("1"); + }); + + it("hides a starred model that is no longer in the available list", async () => { + seedStar("goose", "ghost"); + __resetStarredModelsCacheForTests(); + const user = userEvent.setup(); + render( + , + ); + + await user.click( + screen.getByRole("button", { name: /choose agent and model/i }), + ); + + expect( + document.querySelector('[data-model-key=\'["goose","ghost"]\']'), + ).not.toBeInTheDocument(); + expect( + localStorage.getItem( + starredModelStorageKey(modelStarKey("goose", "ghost")), + ), + ).toBe("1"); + }); }); From 906ba7130d6b09d4b34678d51311522d846a94df Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Fri, 4 Sep 2026 10:09:08 -0400 Subject: [PATCH 08/24] fix(chat): scope star reveal state to each model row The star toggle used unnamed group-hover and group-focus-within variants. Outer picker elements also use the group class, so their hover or focus state could reveal several row stars after the pointer left a row. Name the model-row group and scope both reveal variants to it. This keeps each star visible only while its own row is hovered or focused, or while the star itself has keyboard focus. Co-authored-by: Goose --- src/features/chat/ui/AgentModelPickerLists.tsx | 6 +++--- src/features/chat/ui/__tests__/AgentModelPicker.test.tsx | 6 ++++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/features/chat/ui/AgentModelPickerLists.tsx b/src/features/chat/ui/AgentModelPickerLists.tsx index 372ad8282..bbc9e1143 100644 --- a/src/features/chat/ui/AgentModelPickerLists.tsx +++ b/src/features/chat/ui/AgentModelPickerLists.tsx @@ -496,7 +496,7 @@ export const RecommendedModelList = forwardRef< return (
@@ -533,13 +533,13 @@ export const RecommendedModelList = forwardRef< onClick={() => toggleStar(scopeId, model.id)} // Hover-reveal keeps rows calm; keyboard users still // reach the control through row focus - // (group-focus-within) or direct focus. The idle + // (group-focus-within/model-row) or direct focus. The idle // (unstarred) star rests on the ghost icon contract's // muted-foreground — ≈5.7:1 light / ≈6.1:1 dark against // the popover, above the 3:1 WCAG non-text bar // (enforced in globals.test.ts) — and favorited rows // soften to foreground/80 via the selected flag. - className="shrink-0 opacity-0 transition-opacity group-hover:opacity-100 group-focus-within:opacity-100 focus-visible:opacity-100" + className="shrink-0 opacity-0 transition-opacity group-hover/model-row:opacity-100 group-focus-within/model-row:opacity-100 focus-visible:opacity-100" aria-label={t( starred ? "toolbar.unstarModel" : "toolbar.starModel", { model: getModelDisplayName(model) }, diff --git a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx index 51f51fb3e..e36852d46 100644 --- a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx +++ b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx @@ -1913,6 +1913,12 @@ describe("AgentModelPicker starred models", () => { expect(idleStar).toHaveAttribute("aria-pressed", "false"); expect(idleStar).toHaveClass("text-muted-foreground"); expect(idleStar).not.toHaveClass("text-foreground/80"); + // A named group keeps outer picker hover/focus states from revealing + // every row's star at once. + expect(idleStar).toHaveClass("group-hover/model-row:opacity-100"); + expect(idleStar).toHaveClass("group-focus-within/model-row:opacity-100"); + expect(idleStar).not.toHaveClass("group-hover:opacity-100"); + expect(idleStar).not.toHaveClass("group-focus-within:opacity-100"); const starredToggle = within(picker).getByRole("button", { name: "Unstar Other", From 53aa11387875ac358f2fb1543fef8b6ba52fd888 Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Fri, 4 Sep 2026 11:01:12 -0400 Subject: [PATCH 09/24] fix(chat): clear model star reveal with explicit row state Tauri WebKit can retain CSS :hover after the pointer leaves a row, so named Tailwind groups did not fully fix star buttons hanging around. Track the exact row under the pointer and the row containing keyboard focus in React state instead. Pointer leave from a row or the model column now clears the reveal deterministically. Also keep explicit ghost toggle colors stable on hover: selected=false stays muted-foreground and selected=true stays foreground/80. This removes the mixed light and dark star outlines seen across the list. Co-authored-by: Goose --- .../chat/ui/AgentModelPickerLists.tsx | 40 +++++++++++++++---- .../ui/__tests__/AgentModelPicker.test.tsx | 16 +++++--- .../generated/componentManifest.ts | 9 +++++ src/shared/ui/button.tsx | 12 ++++-- 4 files changed, 61 insertions(+), 16 deletions(-) diff --git a/src/features/chat/ui/AgentModelPickerLists.tsx b/src/features/chat/ui/AgentModelPickerLists.tsx index bbc9e1143..166999c2a 100644 --- a/src/features/chat/ui/AgentModelPickerLists.tsx +++ b/src/features/chat/ui/AgentModelPickerLists.tsx @@ -19,6 +19,7 @@ import { useStarredModels } from "../hooks/useStarredModels"; import { modelStarKey } from "../lib/starredModels"; import { SearchBar } from "@/shared/ui/SearchBar"; import { Button } from "@/shared/ui/button"; +import { cn } from "@/shared/lib/cn"; import { ScrollArea } from "@/shared/ui/scroll-area"; import { Separator } from "@/shared/ui/separator"; import { @@ -204,6 +205,8 @@ export const RecommendedModelList = forwardRef< }, [existingModelKeys, starredKeys]); const [searchOpen, setSearchOpen] = useState(false); const [showAll, setShowAll] = useState(false); + const [hoveredModelKey, setHoveredModelKey] = useState(null); + const [focusedModelKey, setFocusedModelKey] = useState(null); const [query, setQuery] = useState(""); const inputRef = useRef(null); const searchButtonRef = useRef(null); @@ -221,6 +224,8 @@ export const RecommendedModelList = forwardRef< setQuery(""); setSearchOpen(false); setShowAll(false); + setHoveredModelKey(null); + setFocusedModelKey(null); resetScroll(); }, [resetScroll]); const recencyMap = useModelRecency(); @@ -417,7 +422,10 @@ export const RecommendedModelList = forwardRef< }; return ( -
+
setHoveredModelKey(null)} + >
{searchOpen ? (
@@ -496,9 +504,23 @@ export const RecommendedModelList = forwardRef< return (
setHoveredModelKey(modelKey)} + onPointerLeave={() => + setHoveredModelKey((current) => + current === modelKey ? null : current, + ) + } + onFocusCapture={() => setFocusedModelKey(modelKey)} + onBlurCapture={(event) => { + if (!event.currentTarget.contains(event.relatedTarget)) { + setFocusedModelKey((current) => + current === modelKey ? null : current, + ); + } + }} > { @@ -531,15 +553,19 @@ export const RecommendedModelList = forwardRef< size="icon-xs" selected={starred} onClick={() => toggleStar(scopeId, model.id)} - // Hover-reveal keeps rows calm; keyboard users still - // reach the control through row focus - // (group-focus-within/model-row) or direct focus. The idle - // (unstarred) star rests on the ghost icon contract's + // Explicit row-local pointer/focus state avoids + // sticky WebKit :hover state while keeping the list calm. + // The idle (unstarred) star rests on the ghost icon contract's // muted-foreground — ≈5.7:1 light / ≈6.1:1 dark against // the popover, above the 3:1 WCAG non-text bar // (enforced in globals.test.ts) — and favorited rows // soften to foreground/80 via the selected flag. - className="shrink-0 opacity-0 transition-opacity group-hover/model-row:opacity-100 group-focus-within/model-row:opacity-100 focus-visible:opacity-100" + className={cn( + "shrink-0 opacity-0 transition-opacity focus-visible:opacity-100", + (hoveredModelKey === modelKey || + focusedModelKey === modelKey) && + "opacity-100", + )} aria-label={t( starred ? "toolbar.unstarModel" : "toolbar.starModel", { model: getModelDisplayName(model) }, diff --git a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx index e36852d46..6d3e283e0 100644 --- a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx +++ b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx @@ -1912,13 +1912,16 @@ describe("AgentModelPicker starred models", () => { expect(idleStar).toHaveAttribute("data-slot", "button"); expect(idleStar).toHaveAttribute("aria-pressed", "false"); expect(idleStar).toHaveClass("text-muted-foreground"); + expect(idleStar).toHaveClass("hover:text-muted-foreground"); expect(idleStar).not.toHaveClass("text-foreground/80"); - // A named group keeps outer picker hover/focus states from revealing - // every row's star at once. - expect(idleStar).toHaveClass("group-hover/model-row:opacity-100"); - expect(idleStar).toHaveClass("group-focus-within/model-row:opacity-100"); - expect(idleStar).not.toHaveClass("group-hover:opacity-100"); - expect(idleStar).not.toHaveClass("group-focus-within:opacity-100"); + expect(idleStar).toHaveClass("opacity-0"); + + const preferredRow = idleStar.closest("[data-model-key]"); + expect(preferredRow).not.toBeNull(); + await user.hover(preferredRow as HTMLElement); + expect(idleStar).toHaveClass("opacity-100"); + await user.unhover(preferredRow as HTMLElement); + expect(idleStar).not.toHaveClass("opacity-100"); const starredToggle = within(picker).getByRole("button", { name: "Unstar Other", @@ -1926,6 +1929,7 @@ describe("AgentModelPicker starred models", () => { expect(starredToggle).toHaveAttribute("data-slot", "button"); expect(starredToggle).toHaveAttribute("aria-pressed", "true"); expect(starredToggle).toHaveClass("text-foreground/80"); + expect(starredToggle).toHaveClass("hover:text-foreground/80"); expect(starredToggle).not.toHaveClass("text-muted-foreground"); }); diff --git a/src/features/design-system/generated/componentManifest.ts b/src/features/design-system/generated/componentManifest.ts index bce486648..adf3401d2 100644 --- a/src/features/design-system/generated/componentManifest.ts +++ b/src/features/design-system/generated/componentManifest.ts @@ -623,6 +623,7 @@ export const designSystemComponentManifest = [ "hover:text-accent-foreground", "hover:text-destructive", "hover:text-foreground", + "hover:text-foreground/80", "text-accent-foreground", "text-destructive", "text-destructive-foreground", @@ -656,6 +657,7 @@ export const designSystemComponentManifest = [ "hover:text-current", "hover:text-destructive", "hover:text-foreground", + "hover:text-foreground/80", "hover:underline", ], sourceTokenClasses: [ @@ -679,6 +681,7 @@ export const designSystemComponentManifest = [ "hover:text-accent-foreground", "hover:text-destructive", "hover:text-foreground", + "hover:text-foreground/80", "text-accent-foreground", "text-destructive", "text-destructive-foreground", @@ -710,6 +713,8 @@ export const designSystemComponentManifest = [ "hover:text-accent-foreground", "hover:text-destructive", "hover:text-foreground", + "hover:text-foreground/80", + "hover:text-muted-foreground", "text-accent-foreground", "text-destructive", "text-destructive-foreground", @@ -744,6 +749,8 @@ export const designSystemComponentManifest = [ "hover:text-current", "hover:text-destructive", "hover:text-foreground", + "hover:text-foreground/80", + "hover:text-muted-foreground", "hover:underline", ], sourceTokenClasses: [ @@ -767,6 +774,8 @@ export const designSystemComponentManifest = [ "hover:text-accent-foreground", "hover:text-destructive", "hover:text-foreground", + "hover:text-foreground/80", + "hover:text-muted-foreground", "text-accent-foreground", "text-destructive", "text-destructive-foreground", diff --git a/src/shared/ui/button.tsx b/src/shared/ui/button.tsx index 171de69c0..b462f22ca 100644 --- a/src/shared/ui/button.tsx +++ b/src/shared/ui/button.tsx @@ -127,7 +127,7 @@ const buttonVariants = cva( { variant: "ghost", selected: true, - className: "text-foreground/80", + className: "text-foreground/80 hover:text-foreground/80", }, ], defaultVariants: { @@ -310,8 +310,9 @@ export type ButtonProps = React.ButtonHTMLAttributes & * On/pressed state for toggle controls that lean on Button as their * chrome (icon toggles like the model picker star). Ghost buttons rest * the label or icon at foreground/80 — softer than idle content yet - * well above the 3:1 non-text contrast bar — and keep the ghost hover - * at full foreground. Only meaningful on ghost; other variants ignore + * well above the 3:1 non-text contrast bar. Explicit true and false + * states keep their respective color on hover, so toggle groups do not + * show mixed shades. Only meaningful on ghost; other variants ignore * it (dev builds warn). */ selected?: boolean; @@ -573,6 +574,11 @@ const Button = React.forwardRef( selected: resolvedSelected, className, }), + resolvedEmphasis === "ghost" && + selected !== undefined && + (resolvedSelected + ? "text-foreground/80 hover:text-foreground/80" + : "text-muted-foreground hover:text-muted-foreground"), asChild && resolvedDisabled && "pointer-events-none", )} onClick={handleClick} From bc1cca276aec190fe13c4f571836d061e22b2659 Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Fri, 4 Sep 2026 11:10:14 -0400 Subject: [PATCH 10/24] fix(chat): remove trailing fade from model star reveal Explicit row state cleared correctly, but transition-opacity kept each star visible while it faded. Diagonal pointer movement across narrow rows could therefore leave several stars on screen at different shades. Remove the fade so reveal state changes are immediate, and remove the vertical gaps between model-row hit areas so adjacent rows have no dead strip between them. Co-authored-by: Goose --- src/features/chat/ui/AgentModelPickerLists.tsx | 4 ++-- src/features/chat/ui/__tests__/AgentModelPicker.test.tsx | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/features/chat/ui/AgentModelPickerLists.tsx b/src/features/chat/ui/AgentModelPickerLists.tsx index 166999c2a..2f6cd47aa 100644 --- a/src/features/chat/ui/AgentModelPickerLists.tsx +++ b/src/features/chat/ui/AgentModelPickerLists.tsx @@ -481,7 +481,7 @@ export const RecommendedModelList = forwardRef< ref={scrollAreaRef} className="min-h-0 min-w-0 flex-1 [&_[data-slot=scroll-area-viewport]>div]:!block" > -
+
{sorted.map((model, index) => { const providerLabel = getGooseModelProviderLabel(model); const providerIcon = @@ -561,7 +561,7 @@ export const RecommendedModelList = forwardRef< // (enforced in globals.test.ts) — and favorited rows // soften to foreground/80 via the selected flag. className={cn( - "shrink-0 opacity-0 transition-opacity focus-visible:opacity-100", + "shrink-0 opacity-0 focus-visible:opacity-100", (hoveredModelKey === modelKey || focusedModelKey === modelKey) && "opacity-100", diff --git a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx index 6d3e283e0..418628d8e 100644 --- a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx +++ b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx @@ -1915,6 +1915,7 @@ describe("AgentModelPicker starred models", () => { expect(idleStar).toHaveClass("hover:text-muted-foreground"); expect(idleStar).not.toHaveClass("text-foreground/80"); expect(idleStar).toHaveClass("opacity-0"); + expect(idleStar).not.toHaveClass("transition-opacity"); const preferredRow = idleStar.closest("[data-model-key]"); expect(preferredRow).not.toBeNull(); From cf4cda86ab11e1793648e9a60d154673fa5acc67 Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Fri, 4 Sep 2026 11:24:18 -0400 Subject: [PATCH 11/24] fix(chat): restore a quick model star fade Restore a 75 ms opacity transition now that explicit row pointer state and gap-free hit areas prevent stale stars. Keep the stable toggle colors and deterministic row enter/leave behavior unchanged. Co-authored-by: Goose --- src/features/chat/ui/AgentModelPickerLists.tsx | 2 +- src/features/chat/ui/__tests__/AgentModelPicker.test.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/features/chat/ui/AgentModelPickerLists.tsx b/src/features/chat/ui/AgentModelPickerLists.tsx index 2f6cd47aa..e7ba5628b 100644 --- a/src/features/chat/ui/AgentModelPickerLists.tsx +++ b/src/features/chat/ui/AgentModelPickerLists.tsx @@ -561,7 +561,7 @@ export const RecommendedModelList = forwardRef< // (enforced in globals.test.ts) — and favorited rows // soften to foreground/80 via the selected flag. className={cn( - "shrink-0 opacity-0 focus-visible:opacity-100", + "shrink-0 opacity-0 transition-opacity duration-75 focus-visible:opacity-100", (hoveredModelKey === modelKey || focusedModelKey === modelKey) && "opacity-100", diff --git a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx index 418628d8e..9f910fad7 100644 --- a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx +++ b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx @@ -1915,7 +1915,7 @@ describe("AgentModelPicker starred models", () => { expect(idleStar).toHaveClass("hover:text-muted-foreground"); expect(idleStar).not.toHaveClass("text-foreground/80"); expect(idleStar).toHaveClass("opacity-0"); - expect(idleStar).not.toHaveClass("transition-opacity"); + expect(idleStar).toHaveClass("transition-opacity", "duration-75"); const preferredRow = idleStar.closest("[data-model-key]"); expect(preferredRow).not.toBeNull(); From ddad0cace4770c7c9aaa36b9d17dd5fd058f997b Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Fri, 4 Sep 2026 11:25:38 -0400 Subject: [PATCH 12/24] Revert "fix(chat): restore a quick model star fade" This reverts commit cf4cda86ab11e1793648e9a60d154673fa5acc67. Co-authored-by: Goose --- src/features/chat/ui/AgentModelPickerLists.tsx | 2 +- src/features/chat/ui/__tests__/AgentModelPicker.test.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/features/chat/ui/AgentModelPickerLists.tsx b/src/features/chat/ui/AgentModelPickerLists.tsx index e7ba5628b..2f6cd47aa 100644 --- a/src/features/chat/ui/AgentModelPickerLists.tsx +++ b/src/features/chat/ui/AgentModelPickerLists.tsx @@ -561,7 +561,7 @@ export const RecommendedModelList = forwardRef< // (enforced in globals.test.ts) — and favorited rows // soften to foreground/80 via the selected flag. className={cn( - "shrink-0 opacity-0 transition-opacity duration-75 focus-visible:opacity-100", + "shrink-0 opacity-0 focus-visible:opacity-100", (hoveredModelKey === modelKey || focusedModelKey === modelKey) && "opacity-100", diff --git a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx index 9f910fad7..418628d8e 100644 --- a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx +++ b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx @@ -1915,7 +1915,7 @@ describe("AgentModelPicker starred models", () => { expect(idleStar).toHaveClass("hover:text-muted-foreground"); expect(idleStar).not.toHaveClass("text-foreground/80"); expect(idleStar).toHaveClass("opacity-0"); - expect(idleStar).toHaveClass("transition-opacity", "duration-75"); + expect(idleStar).not.toHaveClass("transition-opacity"); const preferredRow = idleStar.closest("[data-model-key]"); expect(preferredRow).not.toBeNull(); From 2efce9c7a1215d4861984d9e834ab470592c2561 Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Fri, 4 Sep 2026 11:36:58 -0400 Subject: [PATCH 13/24] fix(chat): fade model stars in without trailing on exit Use a 75 ms entrance animation instead of transition-opacity. Stars fade in when a row becomes active, but leaving the row removes the animation and opacity class immediately. This keeps the soft reveal without stale stars trailing diagonal pointer movement. Co-authored-by: Goose --- src/features/chat/ui/AgentModelPickerLists.tsx | 2 +- src/features/chat/ui/__tests__/AgentModelPicker.test.tsx | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/features/chat/ui/AgentModelPickerLists.tsx b/src/features/chat/ui/AgentModelPickerLists.tsx index 2f6cd47aa..3243a5b9c 100644 --- a/src/features/chat/ui/AgentModelPickerLists.tsx +++ b/src/features/chat/ui/AgentModelPickerLists.tsx @@ -564,7 +564,7 @@ export const RecommendedModelList = forwardRef< "shrink-0 opacity-0 focus-visible:opacity-100", (hoveredModelKey === modelKey || focusedModelKey === modelKey) && - "opacity-100", + "animate-in fade-in opacity-100 duration-75", )} aria-label={t( starred ? "toolbar.unstarModel" : "toolbar.starModel", diff --git a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx index 418628d8e..e53a6fea9 100644 --- a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx +++ b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx @@ -1920,9 +1920,16 @@ describe("AgentModelPicker starred models", () => { const preferredRow = idleStar.closest("[data-model-key]"); expect(preferredRow).not.toBeNull(); await user.hover(preferredRow as HTMLElement); - expect(idleStar).toHaveClass("opacity-100"); + expect(idleStar).toHaveClass( + "animate-in", + "fade-in", + "opacity-100", + "duration-75", + ); + expect(idleStar).not.toHaveClass("transition-opacity"); await user.unhover(preferredRow as HTMLElement); expect(idleStar).not.toHaveClass("opacity-100"); + expect(idleStar).not.toHaveClass("animate-in", "fade-in"); const starredToggle = within(picker).getByRole("button", { name: "Unstar Other", From f0fbe133d7b38dd79dd73c9e5fec50b6f2bf2a3b Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Fri, 4 Sep 2026 11:42:07 -0400 Subject: [PATCH 14/24] fix(chat): make model star fade-in perceptible Increase the entrance-only star animation from 75 ms to 150 ms. Exit remains immediate, so diagonal pointer movement cannot leave trailing stars. Co-authored-by: Goose --- src/features/chat/ui/AgentModelPickerLists.tsx | 2 +- src/features/chat/ui/__tests__/AgentModelPicker.test.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/features/chat/ui/AgentModelPickerLists.tsx b/src/features/chat/ui/AgentModelPickerLists.tsx index 3243a5b9c..178ca5196 100644 --- a/src/features/chat/ui/AgentModelPickerLists.tsx +++ b/src/features/chat/ui/AgentModelPickerLists.tsx @@ -564,7 +564,7 @@ export const RecommendedModelList = forwardRef< "shrink-0 opacity-0 focus-visible:opacity-100", (hoveredModelKey === modelKey || focusedModelKey === modelKey) && - "animate-in fade-in opacity-100 duration-75", + "animate-in fade-in opacity-100 duration-150", )} aria-label={t( starred ? "toolbar.unstarModel" : "toolbar.starModel", diff --git a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx index e53a6fea9..65a0ba9df 100644 --- a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx +++ b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx @@ -1924,7 +1924,7 @@ describe("AgentModelPicker starred models", () => { "animate-in", "fade-in", "opacity-100", - "duration-75", + "duration-150", ); expect(idleStar).not.toHaveClass("transition-opacity"); await user.unhover(preferredRow as HTMLElement); From 7d25d08e0c6ddcab2068d66115821119baee8850 Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Fri, 4 Sep 2026 11:46:31 -0400 Subject: [PATCH 15/24] fix(chat): keep favorited model stars visible Treat starred state as a reveal condition so favorited models always show their filled star. Unstarred outlines still fade in on row entry and hide immediately on exit. Co-authored-by: Goose --- src/features/chat/ui/AgentModelPickerLists.tsx | 3 ++- src/features/chat/ui/__tests__/AgentModelPicker.test.tsx | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/features/chat/ui/AgentModelPickerLists.tsx b/src/features/chat/ui/AgentModelPickerLists.tsx index 178ca5196..482661a26 100644 --- a/src/features/chat/ui/AgentModelPickerLists.tsx +++ b/src/features/chat/ui/AgentModelPickerLists.tsx @@ -562,7 +562,8 @@ export const RecommendedModelList = forwardRef< // soften to foreground/80 via the selected flag. className={cn( "shrink-0 opacity-0 focus-visible:opacity-100", - (hoveredModelKey === modelKey || + (starred || + hoveredModelKey === modelKey || focusedModelKey === modelKey) && "animate-in fade-in opacity-100 duration-150", )} diff --git a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx index 65a0ba9df..45bbbea44 100644 --- a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx +++ b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx @@ -1938,6 +1938,7 @@ describe("AgentModelPicker starred models", () => { expect(starredToggle).toHaveAttribute("aria-pressed", "true"); expect(starredToggle).toHaveClass("text-foreground/80"); expect(starredToggle).toHaveClass("hover:text-foreground/80"); + expect(starredToggle).toHaveClass("opacity-100", "animate-in", "fade-in"); expect(starredToggle).not.toHaveClass("text-muted-foreground"); }); From 0bcc1f9d1579a330e018521ad22e4173f252cb14 Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Fri, 4 Sep 2026 12:07:44 -0400 Subject: [PATCH 16/24] feat(chat): animate favorite model layout changes Give model rows and the favorites divider stable motion layout items. When a model is starred or unstarred, its row, nearby rows, and the divider slide to their new positions instead of jumping. Use position-only spring animation so row contents do not stretch. Respect reduced-motion preferences by keeping layout changes instant. Co-authored-by: Goose --- .../chat/ui/AgentModelPickerLists.tsx | 49 ++++++++++++++----- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/src/features/chat/ui/AgentModelPickerLists.tsx b/src/features/chat/ui/AgentModelPickerLists.tsx index 482661a26..1951442ab 100644 --- a/src/features/chat/ui/AgentModelPickerLists.tsx +++ b/src/features/chat/ui/AgentModelPickerLists.tsx @@ -15,6 +15,7 @@ import { IconStarFilled, IconX, } from "@tabler/icons-react"; +import { motion, useReducedMotion } from "motion/react"; import { useStarredModels } from "../hooks/useStarredModels"; import { modelStarKey } from "../lib/starredModels"; import { SearchBar } from "@/shared/ui/SearchBar"; @@ -177,6 +178,7 @@ export const RecommendedModelList = forwardRef< ref, ) { const { toggleStar, starredKeys } = useStarredModels(); + const prefersReducedMotion = useReducedMotion(); // Rows include a synthesized entry for the current selection when the // catalog no longer serves it. Honoring starred state only for catalog // models keeps a favorited model a provider dropped from rendering as @@ -379,6 +381,18 @@ export const RecommendedModelList = forwardRef< liveStarredKeys, ]); const sorted = [...grouped.starred, ...grouped.unstarred]; + const layoutItems: Array< + { type: "model"; model: ModelOption } | { type: "favorites-divider" } + > = [ + ...grouped.starred.map((model) => ({ type: "model" as const, model })), + ...(grouped.starred.length > 0 && grouped.unstarred.length > 0 + ? ([{ type: "favorites-divider" }] as const) + : []), + ...grouped.unstarred.map((model) => ({ type: "model" as const, model })), + ]; + const layoutTransition = prefersReducedMotion + ? { duration: 0 } + : { type: "spring" as const, duration: 0.24, bounce: 0 }; const recommendedKeys = new Set( recommended.map((model) => @@ -482,7 +496,23 @@ export const RecommendedModelList = forwardRef< className="min-h-0 min-w-0 flex-1 [&_[data-slot=scroll-area-viewport]>div]:!block" >
- {sorted.map((model, index) => { + {layoutItems.map((item) => { + if (item.type === "favorites-divider") { + return ( + + + + ); + } + + const { model } = item; const providerLabel = getGooseModelProviderLabel(model); const providerIcon = selectedAgentId === "goose" && model.providerId @@ -498,11 +528,12 @@ export const RecommendedModelList = forwardRef< const starred = liveStarredKeys.has(modelKey); const existsInCatalog = !existingModelKeys || existingModelKeys.has(modelKey); - const showStarredDivider = - index === grouped.starred.length - 1 && - grouped.unstarred.length > 0; return ( -
+
) : null}
- {showStarredDivider ? ( - - ) : null} -
+ ); })} {hasMore && !searchOpen && !showAll ? ( From 7d8dd780fe43ec260512ef54e57cad7fcc21664d Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Fri, 4 Sep 2026 14:59:42 -0400 Subject: [PATCH 17/24] feat(chat): stage star toggle animation around row movement Animate star toggles as a three-phase sequence: pop and spin the current star out, move the model row, then spin a newly filled favorite star in. Use one 240 ms rotation per star phase and keep the existing 240 ms row layout spring. Unfavoriting unwinds the filled star before moving the row back. Saved favorites do not animate when the picker opens. Reduced-motion users get an immediate toggle. Delay persistence until the spin-out completes and surface write success so a failed write restores the original row and star. Keep tests aligned with the staged timing, including consecutive toggles and write failures. Co-authored-by: Goose --- src/features/chat/hooks/useStarredModels.ts | 7 +- src/features/chat/lib/starredModels.ts | 14 ++- .../chat/ui/AgentModelPickerLists.tsx | 118 +++++++++++++++++- .../ui/__tests__/AgentModelPicker.test.tsx | 36 ++++-- 4 files changed, 156 insertions(+), 19 deletions(-) diff --git a/src/features/chat/hooks/useStarredModels.ts b/src/features/chat/hooks/useStarredModels.ts index f7ad84964..90b40dfdd 100644 --- a/src/features/chat/hooks/useStarredModels.ts +++ b/src/features/chat/hooks/useStarredModels.ts @@ -58,9 +58,10 @@ export function useStarredModels() { starredKeys.has(modelStarKey(scopeId, modelId)), [starredKeys], ); - const toggleStar = useCallback((scopeId: string, modelId: string) => { - toggleModelStar(scopeId, modelId); - }, []); + const toggleStar = useCallback( + (scopeId: string, modelId: string) => toggleModelStar(scopeId, modelId), + [], + ); return { isStarred, toggleStar, starredKeys }; } diff --git a/src/features/chat/lib/starredModels.ts b/src/features/chat/lib/starredModels.ts index 52a540e5e..2926f30e7 100644 --- a/src/features/chat/lib/starredModels.ts +++ b/src/features/chat/lib/starredModels.ts @@ -85,9 +85,9 @@ function readStarredModels(): StarredModelSet { * other's stars. Note that two windows toggling the same model at the same * instant can still interleave; per-model state stays consistent either way. */ -function persistStarEntry(starKey: string, starred: boolean): void { +function persistStarEntry(starKey: string, starred: boolean): boolean { if (typeof window === "undefined") { - return; + return false; } try { @@ -101,18 +101,21 @@ function persistStarEntry(starKey: string, starred: boolean): void { // The write did not land (storage unavailable or over quota). Tell the // user instead of letting the toggle silently bounce back. toast.error(i18n.t("chat:notifications.starredModelsPersistError")); + window.dispatchEvent(new CustomEvent(STARRED_MODELS_CHANGED_EVENT)); + return false; } window.dispatchEvent(new CustomEvent(STARRED_MODELS_CHANGED_EVENT)); + return true; } export function getStarredModelKeys(): StarredModelSet { return readStarredModels(); } -export function toggleModelStar(scopeId: string, modelId: string): void { +export function toggleModelStar(scopeId: string, modelId: string): boolean { if (typeof window === "undefined") { - return; + return false; } const starKey = modelStarKey(scopeId, modelId); @@ -121,11 +124,12 @@ export function toggleModelStar(scopeId: string, modelId: string): void { migrateLegacyStarredModels(); const starred = window.localStorage.getItem(starredModelStorageKey(starKey)) !== null; - persistStarEntry(starKey, !starred); + return persistStarEntry(starKey, !starred); } catch { // Storage is unavailable, so the toggle cannot be applied at all. The // write path reports its own failures; report this one too. toast.error(i18n.t("chat:notifications.starredModelsPersistError")); + return false; } } diff --git a/src/features/chat/ui/AgentModelPickerLists.tsx b/src/features/chat/ui/AgentModelPickerLists.tsx index 1951442ab..81c82e496 100644 --- a/src/features/chat/ui/AgentModelPickerLists.tsx +++ b/src/features/chat/ui/AgentModelPickerLists.tsx @@ -161,6 +161,22 @@ export interface RecommendedModelListHandle { closeSearch: () => boolean; } +type StarAnimation = { + phase: "out" | "moving" | "in"; + targetStarred: boolean; +}; + +const STAR_SPIN_TRANSITION = { + duration: 0.24, + ease: "easeInOut" as const, + times: [0, 0.18, 0.82, 1], + opacity: { + duration: 0.24, + ease: "easeIn" as const, + times: [0, 0.18, 0.82, 1], + }, +}; + export const RecommendedModelList = forwardRef< RecommendedModelListHandle, ModelListProps @@ -209,7 +225,51 @@ export const RecommendedModelList = forwardRef< const [showAll, setShowAll] = useState(false); const [hoveredModelKey, setHoveredModelKey] = useState(null); const [focusedModelKey, setFocusedModelKey] = useState(null); + const [starAnimation, setStarAnimation] = useState<{ + modelKey: string; + scopeId: string; + modelId: string; + state: StarAnimation; + } | null>(null); const [query, setQuery] = useState(""); + useEffect(() => { + if (!starAnimation || prefersReducedMotion) { + return; + } + if (starAnimation.state.phase === "out") { + const timer = window.setTimeout(() => { + const changed = toggleStar( + starAnimation.scopeId, + starAnimation.modelId, + ); + setHoveredModelKey(null); + setStarAnimation( + changed + ? { + ...starAnimation, + state: { ...starAnimation.state, phase: "moving" }, + } + : null, + ); + }, 240); + return () => window.clearTimeout(timer); + } + if (starAnimation.state.phase === "moving") { + const timer = window.setTimeout(() => { + setStarAnimation( + starAnimation.state.targetStarred + ? { + ...starAnimation, + state: { ...starAnimation.state, phase: "in" }, + } + : null, + ); + }, 240); + return () => window.clearTimeout(timer); + } + const timer = window.setTimeout(() => setStarAnimation(null), 240); + return () => window.clearTimeout(timer); + }, [prefersReducedMotion, starAnimation, toggleStar]); const inputRef = useRef(null); const searchButtonRef = useRef(null); const restoreSearchButtonFocusRef = useRef(false); @@ -393,7 +453,6 @@ export const RecommendedModelList = forwardRef< const layoutTransition = prefersReducedMotion ? { duration: 0 } : { type: "spring" as const, duration: 0.24, bounce: 0 }; - const recommendedKeys = new Set( recommended.map((model) => modelStarKey(model.providerId ?? selectedAgentId, model.id), @@ -528,6 +587,25 @@ export const RecommendedModelList = forwardRef< const starred = liveStarredKeys.has(modelKey); const existsInCatalog = !existingModelKeys || existingModelKeys.has(modelKey); + const activeStarAnimation = + starAnimation?.modelKey === modelKey + ? starAnimation.state + : null; + const handleStarClick = () => { + if (starAnimation) { + return; + } + if (prefersReducedMotion) { + toggleStar(scopeId, model.id); + return; + } + setStarAnimation({ + modelKey, + scopeId, + modelId: model.id, + state: { phase: "out", targetStarred: !starred }, + }); + }; return ( toggleStar(scopeId, model.id)} + onClick={handleStarClick} + data-star-animation-phase={ + activeStarAnimation?.phase ?? undefined + } // Explicit row-local pointer/focus state avoids // sticky WebKit :hover state while keeping the list calm. // The idle (unstarred) star rests on the ghost icon contract's @@ -594,9 +675,13 @@ export const RecommendedModelList = forwardRef< className={cn( "shrink-0 opacity-0 focus-visible:opacity-100", (starred || + activeStarAnimation?.phase === "out" || + activeStarAnimation?.phase === "in" || hoveredModelKey === modelKey || focusedModelKey === modelKey) && "animate-in fade-in opacity-100 duration-150", + activeStarAnimation?.phase === "moving" && + "pointer-events-none opacity-0", )} aria-label={t( starred ? "toolbar.unstarModel" : "toolbar.starModel", @@ -604,7 +689,34 @@ export const RecommendedModelList = forwardRef< )} aria-pressed={starred} > - {starred ? : } + + {starred ? : } + ) : null}
diff --git a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx index 45bbbea44..431acf347 100644 --- a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx +++ b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx @@ -1880,7 +1880,9 @@ describe("AgentModelPicker starred models", () => { ); expect(onModelChange).not.toHaveBeenCalled(); - expect(screen.getByTestId("starred-models-divider")).toBeInTheDocument(); + await waitFor(() => + expect(screen.getByTestId("starred-models-divider")).toBeInTheDocument(), + ); }); it("renders star actions through the shared Button contract with a ≥3:1 idle treatment", async () => { @@ -2003,6 +2005,13 @@ describe("AgentModelPicker starred models", () => { within(picker).getByRole("button", { name: "Star Another" }), ); + await waitFor(() => + expect( + localStorage.getItem( + starredModelStorageKey(modelStarKey("goose", "another")), + ), + ).toBe("1"), + ); // Starring one model must leave every other star entry untouched; an // aggregate rewrite from a stale snapshot would drop "other" here. expect( @@ -2016,15 +2025,22 @@ describe("AgentModelPicker starred models", () => { ), ).toBe("1"); + await waitFor(() => + expect( + picker.querySelector("[data-star-animation-phase]"), + ).not.toBeInTheDocument(), + ); await user.click( within(picker).getByRole("button", { name: "Unstar Other" }), ); - expect( - localStorage.getItem( - starredModelStorageKey(modelStarKey("goose", "other")), - ), - ).toBeNull(); + await waitFor(() => + expect( + localStorage.getItem( + starredModelStorageKey(modelStarKey("goose", "other")), + ), + ).toBeNull(), + ); expect( localStorage.getItem( starredModelStorageKey(modelStarKey("goose", "another")), @@ -2063,7 +2079,9 @@ describe("AgentModelPicker starred models", () => { within(picker).getByRole("button", { name: "Star Another" }), ); - expect(vi.mocked(toast.error)).toHaveBeenCalledTimes(1); + await waitFor(() => + expect(vi.mocked(toast.error)).toHaveBeenCalledTimes(1), + ); expect(vi.mocked(toast.error)).toHaveBeenCalledWith( expect.stringMatching(/starred/i), ); @@ -2111,7 +2129,9 @@ describe("AgentModelPicker starred models", () => { within(picker).getByRole("button", { name: "Unstar Other" }), ); - expect(vi.mocked(toast.error)).toHaveBeenCalledTimes(1); + await waitFor(() => + expect(vi.mocked(toast.error)).toHaveBeenCalledTimes(1), + ); expect(vi.mocked(toast.error)).toHaveBeenCalledWith( expect.stringMatching(/starred/i), ); From 8d892c3345645e7c38537109309d0ed1d39886d2 Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Fri, 4 Sep 2026 15:12:25 -0400 Subject: [PATCH 18/24] fix(chat): keep favorites visible across agent changes Build the favorites section from the combined cached model catalogs for all picker agents while keeping regular rows scoped to the selected agent. Favorites now stay constant when switching between Goose, Claude, Codex, and other ready agents. Track each favorite model's owning agent. Clicking a cross-agent favorite switches to that agent first, then selects the model after the controlled agent state updates. Continue hiding a favorite only when its owning catalog no longer contains the model. Co-authored-by: Goose --- .../ConversationComposerCapability.tsx | 1 + .../chat/hooks/useAgentModelPickerState.ts | 11 ++ .../chat/hooks/useChatSessionController.ts | 2 + .../chat/hooks/useResolvedAgentModelPicker.ts | 2 + src/features/chat/types.ts | 1 + src/features/chat/ui/AgentModelPicker.tsx | 29 ++++- .../chat/ui/AgentModelPickerLists.tsx | 108 ++++++++++++------ src/features/chat/ui/ChatInput.tsx | 2 + src/features/chat/ui/ChatInputToolbar.tsx | 2 + .../ui/__tests__/AgentModelPicker.test.tsx | 52 +++++++++ src/shared/ui/GlobalComposerPill.tsx | 6 + 11 files changed, 180 insertions(+), 36 deletions(-) diff --git a/src/features/chat/capabilities/ConversationComposerCapability.tsx b/src/features/chat/capabilities/ConversationComposerCapability.tsx index cadeb606a..fe1f21242 100644 --- a/src/features/chat/capabilities/ConversationComposerCapability.tsx +++ b/src/features/chat/capabilities/ConversationComposerCapability.tsx @@ -361,6 +361,7 @@ export function ConversationComposerCapability({ currentModel: controller.currentModelName ?? undefined, currentExecutionTarget: controller.currentExecutionTarget, availableModels: controller.availableModels, + favoriteModels: controller.favoriteModels, modelsLoading: controller.modelsLoading, modelStatusMessage: controller.modelStatusMessage, onModelChange: controller.handleModelChange, diff --git a/src/features/chat/hooks/useAgentModelPickerState.ts b/src/features/chat/hooks/useAgentModelPickerState.ts index 6293edcf1..d16c37463 100644 --- a/src/features/chat/hooks/useAgentModelPickerState.ts +++ b/src/features/chat/hooks/useAgentModelPickerState.ts @@ -114,6 +114,16 @@ export function useAgentModelPickerState({ () => getModelsForAgent(selectedAgentId), [getModelsForAgent, selectedAgentId], ); + const favoriteModels = useMemo( + () => + pickerAgents.flatMap((agent) => + getModelsForAgent(agent.id).map((model) => ({ + agentId: agent.id, + model, + })), + ), + [getModelsForAgent, pickerAgents], + ); const providerIdsForSelectedAgent = useMemo( () => @@ -211,6 +221,7 @@ export function useAgentModelPickerState({ selectedAgentId, pickerAgents, availableModels, + favoriteModels, getModelsForAgent, isModelInventoryAuthoritative, modelsLoading, diff --git a/src/features/chat/hooks/useChatSessionController.ts b/src/features/chat/hooks/useChatSessionController.ts index 47d95e529..b8c973c72 100644 --- a/src/features/chat/hooks/useChatSessionController.ts +++ b/src/features/chat/hooks/useChatSessionController.ts @@ -1162,6 +1162,7 @@ export function useChatSessionController({ selectedAgentId, pickerAgents, availableModels, + favoriteModels, getModelsForAgent, modelsLoading, modelStatusMessage, @@ -3556,6 +3557,7 @@ export function useChatSessionController({ currentModelName: effectiveModelSelection?.name ?? null, currentExecutionTarget: session?.executionTarget, availableModels, + favoriteModels, modelsLoading, modelStatusMessage, handleModelChange: handleModelChangeWithContextReset, diff --git a/src/features/chat/hooks/useResolvedAgentModelPicker.ts b/src/features/chat/hooks/useResolvedAgentModelPicker.ts index d21d8a4b9..d2c16b841 100644 --- a/src/features/chat/hooks/useResolvedAgentModelPicker.ts +++ b/src/features/chat/hooks/useResolvedAgentModelPicker.ts @@ -336,6 +336,7 @@ export function useResolvedAgentModelPicker({ const { pickerAgents, availableModels, + favoriteModels, getModelsForAgent, isModelInventoryAuthoritative, modelsLoading, @@ -816,6 +817,7 @@ export function useResolvedAgentModelPicker({ selectedAgentId, pickerAgents, availableModels, + favoriteModels, getModelsForAgent, modelsLoading, modelStatusMessage, diff --git a/src/features/chat/types.ts b/src/features/chat/types.ts index 4313411a1..eb12a47fe 100644 --- a/src/features/chat/types.ts +++ b/src/features/chat/types.ts @@ -150,6 +150,7 @@ export interface ChatInputAgentModelPicker { currentModel?: string; currentExecutionTarget?: SessionExecutionTarget; availableModels?: ModelOption[]; + favoriteModels?: Array<{ agentId: string; model: ModelOption }>; modelsLoading?: boolean; modelStatusMessage?: string | null; onModelChange?: (modelId: string, model?: ModelOption) => void; diff --git a/src/features/chat/ui/AgentModelPicker.tsx b/src/features/chat/ui/AgentModelPicker.tsx index 2069e0be5..8f0f067d0 100644 --- a/src/features/chat/ui/AgentModelPicker.tsx +++ b/src/features/chat/ui/AgentModelPicker.tsx @@ -50,6 +50,7 @@ interface AgentModelPickerProps { currentModelProviderId?: string | null; currentModelName?: string | null; availableModels: ModelOption[]; + favoriteModels?: Array<{ agentId: string; model: ModelOption }>; modelsLoading?: boolean; modelStatusMessage?: string | null; onModelChange?: (modelId: string, model?: ModelOption) => void; @@ -170,6 +171,7 @@ export function AgentModelPicker({ currentModelProviderId = null, currentModelName = null, availableModels, + favoriteModels, modelsLoading = false, modelStatusMessage = null, onModelChange, @@ -366,10 +368,28 @@ export function AgentModelPicker({ } }; - const handleModelSelect = (model: ModelOption) => { - recordModelSelection(selectedAgentId, model); + const pendingCrossAgentModelRef = useRef<{ + agentId: string; + model: ModelOption; + } | null>(null); + const handleModelSelect = (model: ModelOption, agentId: string) => { + if (agentId !== selectedAgentId) { + pendingCrossAgentModelRef.current = { agentId, model }; + onAgentChange(agentId); + return; + } + recordModelSelection(agentId, model); onModelChange?.(model.id, model); }; + useEffect(() => { + const pending = pendingCrossAgentModelRef.current; + if (!pending || pending.agentId !== selectedAgentId) { + return; + } + pendingCrossAgentModelRef.current = null; + recordModelSelection(pending.agentId, pending.model); + onModelChange?.(pending.model.id, pending.model); + }, [onModelChange, selectedAgentId]); // Re-gate the provider column when the popover closes, so every reopen // starts from the compact layout. @@ -703,7 +723,10 @@ export function AgentModelPicker({ key={selectedAgentId} ref={modelListRef} models={displayedModels} - catalogModels={availableModels} + favoriteModels={favoriteModels} + catalogModels={ + favoriteModels?.map(({ model }) => model) ?? availableModels + } currentModelId={currentModelId} currentModelProviderId={currentModelProviderId} selectedAgentId={selectedAgentId} diff --git a/src/features/chat/ui/AgentModelPickerLists.tsx b/src/features/chat/ui/AgentModelPickerLists.tsx index 81c82e496..d8a8c10e6 100644 --- a/src/features/chat/ui/AgentModelPickerLists.tsx +++ b/src/features/chat/ui/AgentModelPickerLists.tsx @@ -134,8 +134,14 @@ function sortModels( }); } +export interface FavoriteModelOption { + agentId: string; + model: ModelOption; +} + interface ModelListProps { models: ModelOption[]; + favoriteModels?: FavoriteModelOption[]; /** * The authoritative catalog the rows were built from, without any * synthesized rows for the current selection. Starred state is only @@ -147,7 +153,7 @@ interface ModelListProps { currentModelId: string | null; currentModelProviderId: string | null; selectedAgentId: string; - onModelSelect: (model: ModelOption) => void; + onModelSelect: (model: ModelOption, agentId: string) => void; /** * Reports whether the list has left the recommended view for the full model * list (search or "View more"), so the picker can hide affordances that @@ -183,6 +189,7 @@ export const RecommendedModelList = forwardRef< >(function RecommendedModelList( { models, + favoriteModels, catalogModels, currentModelId, currentModelProviderId, @@ -195,6 +202,18 @@ export const RecommendedModelList = forwardRef< ) { const { toggleStar, starredKeys } = useStarredModels(); const prefersReducedMotion = useReducedMotion(); + const modelAgentIds = useMemo( + () => + new Map( + (favoriteModels ?? []).map(({ agentId, model }) => [model, agentId]), + ), + [favoriteModels], + ); + const getModelScopeId = useCallback( + (model: ModelOption) => + model.providerId ?? modelAgentIds.get(model) ?? selectedAgentId, + [modelAgentIds, selectedAgentId], + ); // Rows include a synthesized entry for the current selection when the // catalog no longer serves it. Honoring starred state only for catalog // models keeps a favorited model a provider dropped from rendering as @@ -205,22 +224,41 @@ export const RecommendedModelList = forwardRef< } return new Set( catalogModels.map((model) => - modelStarKey(model.providerId ?? selectedAgentId, model.id), + modelStarKey(getModelScopeId(model), model.id), ), ); - }, [catalogModels, selectedAgentId]); + }, [catalogModels, getModelScopeId]); + const favoriteModelKeys = useMemo( + () => + favoriteModels + ? new Set( + favoriteModels.map(({ agentId, model }) => + modelStarKey(model.providerId ?? agentId, model.id), + ), + ) + : existingModelKeys, + [existingModelKeys, favoriteModels], + ); const liveStarredKeys = useMemo(() => { - if (!existingModelKeys) { + if (!favoriteModelKeys) { return starredKeys; } const live = new Set(); for (const key of starredKeys) { - if (existingModelKeys.has(key)) { + if (favoriteModelKeys.has(key)) { live.add(key); } } return live; - }, [existingModelKeys, starredKeys]); + }, [favoriteModelKeys, starredKeys]); + const starredModels = useMemo(() => { + const candidates = + favoriteModels ?? + models.map((model) => ({ agentId: selectedAgentId, model })); + return candidates.filter(({ agentId, model }) => + liveStarredKeys.has(modelStarKey(model.providerId ?? agentId, model.id)), + ); + }, [favoriteModels, liveStarredKeys, models, selectedAgentId]); const [searchOpen, setSearchOpen] = useState(false); const [showAll, setShowAll] = useState(false); const [hoveredModelKey, setHoveredModelKey] = useState(null); @@ -292,11 +330,7 @@ export const RecommendedModelList = forwardRef< }, [resetScroll]); const recencyMap = useModelRecency(); const recommended = useMemo(() => { - const starred = models.filter((model) => - liveStarredKeys.has( - modelStarKey(model.providerId ?? selectedAgentId, model.id), - ), - ); + const starred = starredModels.map(({ model }) => model); const recent = models .map((m) => ({ model: m, @@ -311,10 +345,7 @@ export const RecommendedModelList = forwardRef< currentModelProviderId, ) && !liveStarredKeys.has( - modelStarKey( - entry.model.providerId ?? selectedAgentId, - entry.model.id, - ), + modelStarKey(getModelScopeId(entry.model), entry.model.id), ), ) .sort((left, right) => { @@ -331,9 +362,7 @@ export const RecommendedModelList = forwardRef< .filter( (m) => !recent.some((r) => r.id === m.id && r.providerId === m.providerId) && - !liveStarredKeys.has( - modelStarKey(m.providerId ?? selectedAgentId, m.id), - ), + !liveStarredKeys.has(modelStarKey(getModelScopeId(m), m.id)), ); const shortlist = [...recent, ...rec]; if ( @@ -355,9 +384,7 @@ export const RecommendedModelList = forwardRef< } const unstarredFallback = models.filter( (model) => - !liveStarredKeys.has( - modelStarKey(model.providerId ?? selectedAgentId, model.id), - ), + !liveStarredKeys.has(modelStarKey(getModelScopeId(model), model.id)), ); return [ ...starred, @@ -370,6 +397,8 @@ export const RecommendedModelList = forwardRef< recencyMap, selectedAgentId, liveStarredKeys, + starredModels, + getModelScopeId, ]); useEffect(() => { @@ -398,11 +427,17 @@ export const RecommendedModelList = forwardRef< if (!searchOpen && !showAll) { return recommended; } + const favoriteRows = starredModels.map(({ model }) => model); + const regularRows = models.filter( + (model) => + !liveStarredKeys.has(modelStarKey(getModelScopeId(model), model.id)), + ); + const browsableModels = [...favoriteRows, ...regularRows]; const normalizedQuery = query.trim().toLowerCase(); if (!normalizedQuery) { - return models; + return browsableModels; } - return models.filter( + return browsableModels.filter( (model) => model.name.toLowerCase().includes(normalizedQuery) || model.id.toLowerCase().includes(normalizedQuery) || @@ -410,13 +445,22 @@ export const RecommendedModelList = forwardRef< model.providerName?.toLowerCase().includes(normalizedQuery) || model.providerId?.toLowerCase().includes(normalizedQuery), ); - }, [models, query, recommended, searchOpen, showAll]); + }, [ + liveStarredKeys, + models, + query, + recommended, + searchOpen, + showAll, + starredModels, + getModelScopeId, + ]); const grouped = useMemo(() => { const starred: ModelOption[] = []; const unstarred: ModelOption[] = []; for (const model of visibleModels) { - const scopeId = model.providerId ?? selectedAgentId; + const scopeId = getModelScopeId(model); (liveStarredKeys.has(modelStarKey(scopeId, model.id)) ? starred : unstarred @@ -439,6 +483,7 @@ export const RecommendedModelList = forwardRef< recencyMap, selectedAgentId, liveStarredKeys, + getModelScopeId, ]); const sorted = [...grouped.starred, ...grouped.unstarred]; const layoutItems: Array< @@ -454,15 +499,11 @@ export const RecommendedModelList = forwardRef< ? { duration: 0 } : { type: "spring" as const, duration: 0.24, bounce: 0 }; const recommendedKeys = new Set( - recommended.map((model) => - modelStarKey(model.providerId ?? selectedAgentId, model.id), - ), + recommended.map((model) => modelStarKey(getModelScopeId(model), model.id)), ); const hasMore = models.some( (model) => - !recommendedKeys.has( - modelStarKey(model.providerId ?? selectedAgentId, model.id), - ), + !recommendedKeys.has(modelStarKey(getModelScopeId(model), model.id)), ); const showSearchButton = hasMore || recommended.length > SEARCHABLE_LIST_THRESHOLD; @@ -582,7 +623,8 @@ export const RecommendedModelList = forwardRef< currentModelId, currentModelProviderId, ); - const scopeId = model.providerId ?? selectedAgentId; + const modelAgentId = modelAgentIds.get(model) ?? selectedAgentId; + const scopeId = getModelScopeId(model); const modelKey = modelStarKey(scopeId, model.id); const starred = liveStarredKeys.has(modelKey); const existsInCatalog = @@ -633,7 +675,7 @@ export const RecommendedModelList = forwardRef< > { - onModelSelect(model); + onModelSelect(model, modelAgentId); resetView(); }} selected={isSelected} diff --git a/src/features/chat/ui/ChatInput.tsx b/src/features/chat/ui/ChatInput.tsx index 752d378c4..0f7761abb 100644 --- a/src/features/chat/ui/ChatInput.tsx +++ b/src/features/chat/ui/ChatInput.tsx @@ -341,6 +341,7 @@ export function ChatInput({ currentModelProviderId = null, currentModel, availableModels = [], + favoriteModels, modelsLoading = false, modelStatusMessage = null, onModelChange, @@ -1966,6 +1967,7 @@ export function ChatInput({ currentModelProviderId, currentModel: resolvedCurrentModel, availableModels, + favoriteModels, modelsLoading, modelStatusMessage, onModelChange, diff --git a/src/features/chat/ui/ChatInputToolbar.tsx b/src/features/chat/ui/ChatInputToolbar.tsx index 62cf71e43..aefafe045 100644 --- a/src/features/chat/ui/ChatInputToolbar.tsx +++ b/src/features/chat/ui/ChatInputToolbar.tsx @@ -114,6 +114,7 @@ export function ChatInputToolbar({ currentModelProviderId, currentModel, availableModels = [], + favoriteModels, modelsLoading = false, modelStatusMessage = null, onModelChange, @@ -357,6 +358,7 @@ export function ChatInputToolbar({ currentModelProviderId={currentModelProviderId} currentModelName={currentModel ?? null} availableModels={availableModels} + favoriteModels={favoriteModels} modelsLoading={modelsLoading} modelStatusMessage={modelStatusMessage} onModelChange={onModelChange} diff --git a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx index 431acf347..701bc12de 100644 --- a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx +++ b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx @@ -2232,4 +2232,56 @@ describe("AgentModelPicker starred models", () => { ), ).toBe("1"); }); + + it("keeps favorites from other agents visible and switches before selecting", async () => { + seedStar("claude-acp", "opus"); + __resetStarredModelsCacheForTests(); + const user = userEvent.setup(); + const onAgentChange = vi.fn(); + const onModelChange = vi.fn(); + const favoriteModels = [ + ...models.map((model) => ({ agentId: "goose", model })), + { + agentId: "claude-acp", + model: { id: "opus", name: "Claude Opus" }, + }, + ]; + const { rerender } = render( + , + ); + + await user.click( + screen.getByRole("button", { name: /choose agent and model/i }), + ); + const picker = screen.getByRole("dialog"); + expect(within(picker).getByText("Claude Opus")).toBeInTheDocument(); + await user.click(within(picker).getByText("Claude Opus")); + expect(onAgentChange).toHaveBeenCalledWith("claude-acp"); + expect(onModelChange).not.toHaveBeenCalled(); + + rerender( + , + ); + expect(onModelChange).toHaveBeenCalledWith( + "opus", + expect.objectContaining({ id: "opus" }), + ); + }); }); diff --git a/src/shared/ui/GlobalComposerPill.tsx b/src/shared/ui/GlobalComposerPill.tsx index b115ac188..3d7a526d7 100644 --- a/src/shared/ui/GlobalComposerPill.tsx +++ b/src/shared/ui/GlobalComposerPill.tsx @@ -1513,6 +1513,12 @@ export function GlobalComposerPill({ } currentModelName={effectiveModelSelection?.modelName ?? null} availableModels={availableModels} + favoriteModels={pickerAgents.flatMap((agent) => + getModelsForAgent(agent.id).map((model) => ({ + agentId: agent.id, + model, + })), + )} modelsLoading={modelsLoading} modelStatusMessage={modelStatusMessage} onModelChange={handleModelChange} From 2c9bfc5040ca165c7c7635db9c1f0d3ee3b120fb Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Fri, 4 Sep 2026 15:15:16 -0400 Subject: [PATCH 19/24] fix(chat): show owning agent icons on cross-agent favorites Render the owning agent's icon for favorite rows outside Goose. Preserve Goose's existing behavior: show model-provider icons when providerId is present, and leave providerless Goose rows unchanged. Add a regression assertion that a Claude favorite shown while Goose is selected includes the Claude icon. The test fails before the render fix. Co-authored-by: Goose --- src/features/chat/ui/AgentModelPickerLists.tsx | 15 +++++++++++---- .../chat/ui/__tests__/AgentModelPicker.test.tsx | 8 +++++++- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/features/chat/ui/AgentModelPickerLists.tsx b/src/features/chat/ui/AgentModelPickerLists.tsx index d8a8c10e6..b07136975 100644 --- a/src/features/chat/ui/AgentModelPickerLists.tsx +++ b/src/features/chat/ui/AgentModelPickerLists.tsx @@ -613,17 +613,24 @@ export const RecommendedModelList = forwardRef< } const { model } = item; - const providerLabel = getGooseModelProviderLabel(model); + const modelAgentId = modelAgentIds.get(model) ?? selectedAgentId; + const iconProviderId = + modelAgentId === "goose" && model.providerId + ? model.providerId + : modelAgentId; + const providerLabel = + modelAgentId === "goose" + ? getGooseModelProviderLabel(model) + : formatProviderLabel(modelAgentId); const providerIcon = - selectedAgentId === "goose" && model.providerId - ? getProviderIcon(model.providerId, "size-3.5") + modelAgentId !== "goose" || model.providerId + ? getProviderIcon(iconProviderId, "size-3.5") : null; const isSelected = modelMatchesSelection( model, currentModelId, currentModelProviderId, ); - const modelAgentId = modelAgentIds.get(model) ?? selectedAgentId; const scopeId = getModelScopeId(model); const modelKey = modelStarKey(scopeId, model.id); const starred = liveStarredKeys.has(modelKey); diff --git a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx index 701bc12de..7d8fbd704 100644 --- a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx +++ b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx @@ -2263,7 +2263,13 @@ describe("AgentModelPicker starred models", () => { screen.getByRole("button", { name: /choose agent and model/i }), ); const picker = screen.getByRole("dialog"); - expect(within(picker).getByText("Claude Opus")).toBeInTheDocument(); + const claudeFavorite = within(picker) + .getByText("Claude Opus") + .closest("[data-model-key]"); + expect(claudeFavorite).toBeInTheDocument(); + expect( + within(claudeFavorite as HTMLElement).getByTitle("Claude"), + ).toBeInTheDocument(); await user.click(within(picker).getByText("Claude Opus")); expect(onAgentChange).toHaveBeenCalledWith("claude-acp"); expect(onModelChange).not.toHaveBeenCalled(); From eeecf35105c7ba75181affab1692668d0665af02 Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Fri, 4 Sep 2026 16:12:42 -0400 Subject: [PATCH 20/24] fix(chat): stabilize favorite animations and picker layout Keep favorite hover state stable while rows move, transfer hover to the row that finishes under a stationary pointer, and collapse cross-agent favorites that have no destination in the selected agent catalog. Prevent duplicate Claude default rows and sort favorites alphabetically across agents. Gate the agent panel by default, keep Switch agent anchored in the footer, fix the picker width for each panel mode, and anchor smart placement to the trigger's leading edge so model label changes do not move the popover. Co-authored-by: Goose --- src/features/chat/ui/AgentModelPicker.tsx | 54 ++--- .../chat/ui/AgentModelPickerLists.tsx | 206 ++++++++++++----- .../ui/__tests__/AgentModelPicker.test.tsx | 208 +++++++++++++++--- 3 files changed, 352 insertions(+), 116 deletions(-) diff --git a/src/features/chat/ui/AgentModelPicker.tsx b/src/features/chat/ui/AgentModelPicker.tsx index 8f0f067d0..eb096c744 100644 --- a/src/features/chat/ui/AgentModelPicker.tsx +++ b/src/features/chat/ui/AgentModelPicker.tsx @@ -80,8 +80,6 @@ type PopoverContentAlign = NonNullable< ComponentProps["align"] >; const REASONING_EFFORT_COLUMN_TRANSITION_MS = 240; -const PICKER_WIDTH_COMPACT_PX = 452; -const PICKER_WIDTH_EXPANDED_PX = 628; function toSentenceCaseLabel(value: string | undefined): string { const trimmed = value?.trim(); @@ -187,7 +185,7 @@ export function AgentModelPicker({ reasoningEffort, contentAlign = "start", contentCollisionPadding = 16, - providerColumnMode = "visible", + providerColumnMode = "gated", }: AgentModelPickerProps) { const { t } = useTranslation("chat"); const [uncontrolledOpen, setUncontrolledOpen] = useState(false); @@ -211,7 +209,6 @@ export function AgentModelPicker({ }); const modelListRef = useRef(null); const [providerRevealed, setProviderRevealed] = useState(false); - const [modelBrowsing, setModelBrowsing] = useState(false); const [resolvedContentAlign, setResolvedContentAlign] = useState("start"); const [latchedReasoningEffortConfig, setLatchedReasoningEffortConfig] = @@ -396,29 +393,19 @@ export function AgentModelPicker({ useEffect(() => { if (!open) { setProviderRevealed(false); - setModelBrowsing(false); } }, [open]); const showAgentColumn = providerColumnMode === "visible" || providerRevealed; - // A sole ready agent leaves nothing to reveal, but a sole not-ready agent - // still needs the footer: the hidden column's Connect/Install row is the - // only setup path from this picker. - const hasAgentNeedingSetup = agents.some( - (agent) => agent.readiness && agent.readiness !== "ready", - ); - // Browsing the full model list (search or "View more") is a model-picking - // task; the reveal button would swap the whole popover out from under it. + // Keep the reveal action anchored below the scrolling model area whenever + // the panel is gated. Agent discovery may temporarily report one agent; it + // must not remove the user's route to the full agent panel. const showSwitchProviderFooter = - providerColumnMode === "gated" && - !providerRevealed && - !modelBrowsing && - (agents.length > 1 || hasAgentNeedingSetup); + providerColumnMode === "gated" && !providerRevealed; const showReasoningEffortColumn = showReasoningEffort; - const isWidePicker = showReasoningEffortColumn && showAgentColumn; - const pickerWidth = isWidePicker - ? PICKER_WIDTH_EXPANDED_PX - : PICKER_WIDTH_COMPACT_PX; + // Model changes can add or remove reasoning controls. Keep the popover width + // fixed for the current panel mode so those changes do not resize it. + const isWidePicker = showAgentColumn; // Land keyboard focus in the revealed column, since the reveal button that // held focus unmounts with it. @@ -435,20 +422,12 @@ export function AgentModelPicker({ }, [providerRevealed]); const resolveContentAlign = useCallback((): PopoverContentAlign => { - if (contentAlign !== "smart") { - return contentAlign; - } - - const triggerRect = triggerRef.current?.getBoundingClientRect(); - if (!triggerRect) { - return "start"; - } - - const leftAlignedRightEdge = triggerRect.left + pickerWidth; - return leftAlignedRightEdge <= window.innerWidth - contentCollisionPadding - ? "start" - : "center"; - }, [contentAlign, contentCollisionPadding, pickerWidth]); + // Center alignment follows the trigger's center, so a model label changing + // the trigger width makes the open popover jump left or right. Anchor smart + // placement to the trigger's stable leading edge instead; Radix still + // shifts the content when needed to keep it inside the viewport. + return contentAlign === "smart" ? "start" : contentAlign; + }, [contentAlign]); useEffect(() => { if (open) { @@ -724,14 +703,11 @@ export function AgentModelPicker({ ref={modelListRef} models={displayedModels} favoriteModels={favoriteModels} - catalogModels={ - favoriteModels?.map(({ model }) => model) ?? availableModels - } + catalogModels={availableModels} currentModelId={currentModelId} currentModelProviderId={currentModelProviderId} selectedAgentId={selectedAgentId} onModelSelect={handleModelSelect} - onBrowseChange={setModelBrowsing} t={t} /> ) : ( diff --git a/src/features/chat/ui/AgentModelPickerLists.tsx b/src/features/chat/ui/AgentModelPickerLists.tsx index b07136975..df4cacdc5 100644 --- a/src/features/chat/ui/AgentModelPickerLists.tsx +++ b/src/features/chat/ui/AgentModelPickerLists.tsx @@ -60,6 +60,24 @@ function getGooseModelProviderLabel(model: ModelOption) { return null; } +function compareModelsAlphabetically(left: ModelOption, right: ModelOption) { + const byName = getModelDisplayName(left).localeCompare( + getModelDisplayName(right), + undefined, + { sensitivity: "base" }, + ); + if (byName !== 0) { + return byName; + } + + const byId = left.id.localeCompare(right.id); + if (byId !== 0) { + return byId; + } + + return (left.providerId ?? "").localeCompare(right.providerId ?? ""); +} + function compareModelsByProviderOrderAndName( left: ModelOption, right: ModelOption, @@ -239,6 +257,13 @@ export const RecommendedModelList = forwardRef< : existingModelKeys, [existingModelKeys, favoriteModels], ); + const [starAnimation, setStarAnimation] = useState<{ + modelKey: string; + scopeId: string; + modelId: string; + hasSelectedAgentDestination: boolean; + state: StarAnimation; + } | null>(null); const liveStarredKeys = useMemo(() => { if (!favoriteModelKeys) { return starredKeys; @@ -255,21 +280,41 @@ export const RecommendedModelList = forwardRef< const candidates = favoriteModels ?? models.map((model) => ({ agentId: selectedAgentId, model })); - return candidates.filter(({ agentId, model }) => - liveStarredKeys.has(modelStarKey(model.providerId ?? agentId, model.id)), - ); - }, [favoriteModels, liveStarredKeys, models, selectedAgentId]); + return candidates.filter(({ agentId, model }) => { + const modelKey = modelStarKey(model.providerId ?? agentId, model.id); + return ( + liveStarredKeys.has(modelKey) || + (starAnimation?.modelKey === modelKey && + !starAnimation.hasSelectedAgentDestination && + starAnimation.state.phase === "moving" && + !starAnimation.state.targetStarred) + ); + }); + }, [favoriteModels, liveStarredKeys, models, selectedAgentId, starAnimation]); const [searchOpen, setSearchOpen] = useState(false); const [showAll, setShowAll] = useState(false); const [hoveredModelKey, setHoveredModelKey] = useState(null); const [focusedModelKey, setFocusedModelKey] = useState(null); - const [starAnimation, setStarAnimation] = useState<{ - modelKey: string; - scopeId: string; - modelId: string; - state: StarAnimation; - } | null>(null); const [query, setQuery] = useState(""); + const pointerPositionRef = useRef<{ x: number; y: number } | null>(null); + const rowElementsRef = useRef(new Map()); + const reconcileRowHover = useCallback(() => { + const pointer = pointerPositionRef.current; + if (!pointer) { + setHoveredModelKey(null); + return; + } + const hoveredEntry = Array.from(rowElementsRef.current).find(([, row]) => { + const bounds = row.getBoundingClientRect(); + return ( + pointer.x >= bounds.left && + pointer.x <= bounds.right && + pointer.y >= bounds.top && + pointer.y <= bounds.bottom + ); + }); + setHoveredModelKey(hoveredEntry?.[0] ?? null); + }, []); useEffect(() => { if (!starAnimation || prefersReducedMotion) { return; @@ -280,7 +325,6 @@ export const RecommendedModelList = forwardRef< starAnimation.scopeId, starAnimation.modelId, ); - setHoveredModelKey(null); setStarAnimation( changed ? { @@ -294,20 +338,24 @@ export const RecommendedModelList = forwardRef< } if (starAnimation.state.phase === "moving") { const timer = window.setTimeout(() => { - setStarAnimation( - starAnimation.state.targetStarred - ? { - ...starAnimation, - state: { ...starAnimation.state, phase: "in" }, - } - : null, - ); + if (starAnimation.state.targetStarred) { + setStarAnimation({ + ...starAnimation, + state: { ...starAnimation.state, phase: "in" }, + }); + } else { + reconcileRowHover(); + setStarAnimation(null); + } }, 240); return () => window.clearTimeout(timer); } - const timer = window.setTimeout(() => setStarAnimation(null), 240); + const timer = window.setTimeout(() => { + reconcileRowHover(); + setStarAnimation(null); + }, 240); return () => window.clearTimeout(timer); - }, [prefersReducedMotion, starAnimation, toggleStar]); + }, [prefersReducedMotion, reconcileRowHover, starAnimation, toggleStar]); const inputRef = useRef(null); const searchButtonRef = useRef(null); const restoreSearchButtonFocusRef = useRef(false); @@ -461,16 +509,19 @@ export const RecommendedModelList = forwardRef< const unstarred: ModelOption[] = []; for (const model of visibleModels) { const scopeId = getModelScopeId(model); - (liveStarredKeys.has(modelStarKey(scopeId, model.id)) + const modelKey = modelStarKey(scopeId, model.id); + const retainedForeignFavorite = + starAnimation?.modelKey === modelKey && + !starAnimation.hasSelectedAgentDestination && + starAnimation.state.phase === "moving" && + !starAnimation.state.targetStarred; + (liveStarredKeys.has(modelKey) || retainedForeignFavorite ? starred : unstarred ).push(model); } return { - starred: sortModels(starred, currentModelId, currentModelProviderId, { - map: recencyMap, - agentId: selectedAgentId, - }), + starred: [...starred].sort(compareModelsAlphabetically), unstarred: sortModels(unstarred, currentModelId, currentModelProviderId, { map: recencyMap, agentId: selectedAgentId, @@ -484,6 +535,7 @@ export const RecommendedModelList = forwardRef< selectedAgentId, liveStarredKeys, getModelScopeId, + starAnimation, ]); const sorted = [...grouped.starred, ...grouped.unstarred]; const layoutItems: Array< @@ -635,11 +687,15 @@ export const RecommendedModelList = forwardRef< const modelKey = modelStarKey(scopeId, model.id); const starred = liveStarredKeys.has(modelKey); const existsInCatalog = - !existingModelKeys || existingModelKeys.has(modelKey); + !favoriteModelKeys || favoriteModelKeys.has(modelKey); const activeStarAnimation = starAnimation?.modelKey === modelKey ? starAnimation.state : null; + const idleStarVisible = + starred || + hoveredModelKey === modelKey || + focusedModelKey === modelKey; const handleStarClick = () => { if (starAnimation) { return; @@ -652,6 +708,8 @@ export const RecommendedModelList = forwardRef< modelKey, scopeId, modelId: model.id, + hasSelectedAgentDestination: + existingModelKeys?.has(modelKey) ?? true, state: { phase: "out", targetStarred: !starred }, }); }; @@ -659,18 +717,56 @@ export const RecommendedModelList = forwardRef<
{ + if (element) { + rowElementsRef.current.set(modelKey, element); + } else { + rowElementsRef.current.delete(modelKey); + } + }} className="flex min-w-0 items-center gap-1" data-model-key={modelKey} data-starred={starred || undefined} - onPointerEnter={() => setHoveredModelKey(modelKey)} - onPointerLeave={() => - setHoveredModelKey((current) => - current === modelKey ? null : current, - ) - } + onPointerMove={(event) => { + pointerPositionRef.current = { + x: event.clientX, + y: event.clientY, + }; + }} + onPointerEnter={(event) => { + pointerPositionRef.current = { + x: event.clientX, + y: event.clientY, + }; + if (starAnimation?.modelKey !== modelKey) { + setHoveredModelKey(modelKey); + } + }} + onPointerLeave={(event) => { + pointerPositionRef.current = { + x: event.clientX, + y: event.clientY, + }; + if (starAnimation?.modelKey !== modelKey) { + setHoveredModelKey((current) => + current === modelKey ? null : current, + ); + } + }} onFocusCapture={() => setFocusedModelKey(modelKey)} onBlurCapture={(event) => { if (!event.currentTarget.contains(event.relatedTarget)) { @@ -722,15 +818,9 @@ export const RecommendedModelList = forwardRef< // (enforced in globals.test.ts) — and favorited rows // soften to foreground/80 via the selected flag. className={cn( - "shrink-0 opacity-0 focus-visible:opacity-100", - (starred || - activeStarAnimation?.phase === "out" || - activeStarAnimation?.phase === "in" || - hoveredModelKey === modelKey || - focusedModelKey === modelKey) && - "animate-in fade-in opacity-100 duration-150", + "shrink-0", activeStarAnimation?.phase === "moving" && - "pointer-events-none opacity-0", + "pointer-events-none", )} aria-label={t( starred ? "toolbar.unstarModel" : "toolbar.starModel", @@ -750,19 +840,31 @@ export const RecommendedModelList = forwardRef< scale: [1, 0.78, 1.18, 0.9], opacity: [1, 1, 0.7, 0], } - : activeStarAnimation?.phase === "in" + : activeStarAnimation?.phase === "moving" ? { - rotate: [-360, -180, 0, 0], - scale: [0.9, 1.18, 0.96, 1], - opacity: [0, 0, 0.7, 1], - } - : { - rotate: 0, - scale: 1, - opacity: 1, + rotate: -360, + scale: 0.9, + opacity: 0, } + : activeStarAnimation?.phase === "in" + ? { + rotate: [-360, -180, 0, 0], + scale: [0.9, 1.18, 0.96, 1], + opacity: [0, 0, 0.7, 1], + } + : { + rotate: 0, + scale: 1, + opacity: idleStarVisible ? 1 : 0, + } + } + transition={ + activeStarAnimation + ? STAR_SPIN_TRANSITION + : idleStarVisible && !starred + ? { opacity: { duration: 0.15 } } + : { opacity: { duration: 0 } } } - transition={STAR_SPIN_TRANSITION} > {starred ? : } diff --git a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx index 7d8fbd704..43a2239e5 100644 --- a/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx +++ b/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx @@ -94,6 +94,7 @@ describe("AgentModelPicker", () => { onAgentChange={onAgentChange} availableModels={[]} onModelChange={vi.fn()} + providerColumnMode="visible" onRequestComposerFocus={onRequestComposerFocus} />, ); @@ -138,6 +139,7 @@ describe("AgentModelPicker", () => { onAgentChange={onAgentChange} availableModels={[]} onModelChange={vi.fn()} + providerColumnMode="visible" />, ); @@ -527,6 +529,10 @@ describe("AgentModelPicker", () => { ); expect(screen.getByText("Reasoning effort")).toBeInTheDocument(); + const picker = screen.getByRole("dialog"); + const initialWidthClass = Array.from(picker.classList).find((className) => + className.startsWith("w-[min("), + ); rerender( { }, { timeout: 500 }, ); + expect( + Array.from(picker.classList).find((className) => + className.startsWith("w-[min("), + ), + ).toBe(initialWidthClass); }); it("passes the clicked model option through for duplicate model ids", async () => { @@ -1011,6 +1022,7 @@ describe("AgentModelPicker", () => { { id: "gpt-4o-mini", name: "GPT-4o mini" }, ]} onModelChange={vi.fn()} + providerColumnMode="visible" />, ); @@ -1081,6 +1093,7 @@ describe("AgentModelPicker", () => { { id: "gpt-4o-mini", name: "GPT-4o mini" }, ]} onModelChange={vi.fn()} + providerColumnMode="visible" />, ); @@ -1307,7 +1320,7 @@ describe("AgentModelPicker", () => { ).toHaveFocus(); }); - it("hides the switch-agent button while searching models", async () => { + it("keeps the switch-agent footer while searching models", async () => { const user = userEvent.setup(); renderGated({ availableModels: BROWSABLE_MODELS }); @@ -1315,8 +1328,8 @@ describe("AgentModelPicker", () => { await user.click(screen.getByRole("button", { name: /search models/i })); expect( - screen.queryByRole("button", { name: /switch agent/i }), - ).toBeNull(); + screen.getByRole("button", { name: /switch agent/i }), + ).toBeInTheDocument(); await user.keyboard("{Escape}"); @@ -1325,7 +1338,7 @@ describe("AgentModelPicker", () => { ).toBeInTheDocument(); }); - it("hides the switch-agent button while browsing all models", async () => { + it("keeps the switch-agent footer while browsing all models", async () => { const user = userEvent.setup(); renderGated({ availableModels: BROWSABLE_MODELS }); @@ -1333,8 +1346,8 @@ describe("AgentModelPicker", () => { await user.click(screen.getByRole("button", { name: /view more/i })); expect( - screen.queryByRole("button", { name: /switch agent/i }), - ).toBeNull(); + screen.getByRole("button", { name: /switch agent/i }), + ).toBeInTheDocument(); await user.keyboard("{Escape}"); await waitFor(() => { @@ -1436,15 +1449,15 @@ describe("AgentModelPicker", () => { expect(content).toHaveClass("w-[min(39.25rem,calc(100vw-1.5rem))]"); }); - it("hides the switch-agent button when the only agent is ready", async () => { + it("keeps the switch-agent footer during partial agent discovery", async () => { const user = userEvent.setup(); renderGated({ agents: [{ id: "goose", label: "Goose" }] }); await openPicker(user); expect( - screen.queryByRole("button", { name: /switch agent/i }), - ).toBeNull(); + screen.getByRole("button", { name: /switch agent/i }), + ).toBeInTheDocument(); }); it("keeps the switch-agent button when the only agent needs setup", async () => { @@ -1480,7 +1493,7 @@ describe("AgentModelPicker", () => { window.removeEventListener(OPEN_SETTINGS_EVENT, openSettings); }); - it("keeps the agent column visible by default", async () => { + it("hides the agent column behind Switch agent by default", async () => { const user = userEvent.setup(); render( @@ -1497,13 +1510,18 @@ describe("AgentModelPicker", () => { await openPicker(user); + expect(document.querySelector('[data-col="agent"]')).toHaveAttribute( + "data-hidden", + "true", + ); + expect(screen.queryByRole("button", { name: "Claude Code" })).toBeNull(); + + await user.click(screen.getByRole("button", { name: /switch agent/i })); + expect(document.querySelector('[data-col="agent"]')).toHaveAttribute( "data-hidden", "false", ); - expect( - screen.queryByRole("button", { name: /switch agent/i }), - ).toBeNull(); expect( screen.getByRole("button", { name: "Claude Code" }), ).toBeInTheDocument(); @@ -1916,22 +1934,23 @@ describe("AgentModelPicker starred models", () => { expect(idleStar).toHaveClass("text-muted-foreground"); expect(idleStar).toHaveClass("hover:text-muted-foreground"); expect(idleStar).not.toHaveClass("text-foreground/80"); - expect(idleStar).toHaveClass("opacity-0"); - expect(idleStar).not.toHaveClass("transition-opacity"); + expect(idleStar).not.toHaveClass( + "opacity-0", + "opacity-100", + "transition-opacity", + "animate-in", + "fade-in", + ); const preferredRow = idleStar.closest("[data-model-key]"); expect(preferredRow).not.toBeNull(); + const idleStarIcon = idleStar.firstElementChild; + expect(idleStarIcon).toBeInTheDocument(); await user.hover(preferredRow as HTMLElement); - expect(idleStar).toHaveClass( - "animate-in", - "fade-in", - "opacity-100", - "duration-150", - ); - expect(idleStar).not.toHaveClass("transition-opacity"); - await user.unhover(preferredRow as HTMLElement); - expect(idleStar).not.toHaveClass("opacity-100"); + expect(idleStar.firstElementChild).toBe(idleStarIcon); expect(idleStar).not.toHaveClass("animate-in", "fade-in"); + await user.unhover(preferredRow as HTMLElement); + expect(idleStar.firstElementChild).toBe(idleStarIcon); const starredToggle = within(picker).getByRole("button", { name: "Unstar Other", @@ -1940,10 +1959,50 @@ describe("AgentModelPicker starred models", () => { expect(starredToggle).toHaveAttribute("aria-pressed", "true"); expect(starredToggle).toHaveClass("text-foreground/80"); expect(starredToggle).toHaveClass("hover:text-foreground/80"); - expect(starredToggle).toHaveClass("opacity-100", "animate-in", "fade-in"); + expect(starredToggle).not.toHaveClass( + "opacity-0", + "opacity-100", + "animate-in", + "fade-in", + ); expect(starredToggle).not.toHaveClass("text-muted-foreground"); }); + it("does not restart the hover fade during a star click animation", async () => { + const user = userEvent.setup(); + render( + , + ); + + await user.click( + screen.getByRole("button", { name: /choose agent and model/i }), + ); + const star = screen.getByRole("button", { name: "Star Preferred" }); + const row = star.closest("[data-model-key]"); + expect(row).not.toBeNull(); + const starIcon = star.firstElementChild; + await user.hover(row as HTMLElement); + expect(star.firstElementChild).toBe(starIcon); + + await user.click(star); + expect(star).toHaveAttribute("data-star-animation-phase", "out"); + expect(star.firstElementChild).toBe(starIcon); + expect(star).not.toHaveClass( + "opacity-0", + "opacity-100", + "animate-in", + "fade-in", + ); + }); + it("migrates the legacy aggregate entry into per-key entries", async () => { localStorage.setItem( LEGACY_STARRED_MODELS_STORAGE_KEY, @@ -2233,6 +2292,105 @@ describe("AgentModelPicker starred models", () => { ).toBe("1"); }); + it("sorts favorites alphabetically across agents and providers", async () => { + seedStar("claude-acp", "zebra"); + seedStar("goose", "alpha"); + seedStar("codex-acp", "middle"); + __resetStarredModelsCacheForTests(); + const user = userEvent.setup(); + const favoriteModels = [ + { + agentId: "claude-acp", + model: { id: "zebra", name: "zebra" }, + }, + { + agentId: "goose", + model: { id: "alpha", name: "Alpha", providerId: "goose" }, + }, + { + agentId: "codex-acp", + model: { id: "middle", name: "Middle" }, + }, + ]; + render( + , + ); + + await user.click( + screen.getByRole("button", { name: /choose agent and model/i }), + ); + const picker = screen.getByRole("dialog"); + const favoriteKeys = Array.from( + picker.querySelectorAll('[data-starred="true"]'), + ).map((row) => row.getAttribute("data-model-key")); + expect(favoriteKeys).toEqual([ + modelStarKey("goose", "alpha"), + modelStarKey("codex-acp", "middle"), + modelStarKey("claude-acp", "zebra"), + ]); + }); + + it("keeps one stable row when unstarring Claude default", async () => { + seedStar("claude-acp", "default"); + __resetStarredModelsCacheForTests(); + const user = userEvent.setup(); + const selectedModels = [ + { id: "default", name: "Default" }, + { id: "sonnet", name: "Sonnet" }, + ]; + const favoriteModels = [ + { + agentId: "claude-acp", + // A distinct object mirrors the combined-catalog copy used by the app. + model: { id: "default", name: "Default" }, + }, + ]; + render( + , + ); + + await user.click( + screen.getByRole("button", { name: /choose agent and model/i }), + ); + const picker = screen.getByRole("dialog"); + await user.click( + within(picker).getByRole("button", { name: "Unstar Default" }), + ); + await waitFor(() => + expect( + localStorage.getItem( + starredModelStorageKey(modelStarKey("claude-acp", "default")), + ), + ).toBeNull(), + ); + + expect( + Array.from(picker.querySelectorAll("[data-model-key]")).filter( + (row) => + row.getAttribute("data-model-key") === + modelStarKey("claude-acp", "default"), + ), + ).toHaveLength(1); + }); + it("keeps favorites from other agents visible and switches before selecting", async () => { seedStar("claude-acp", "opus"); __resetStarredModelsCacheForTests(); From 34343e33feac4c9e302ae25e1f5323e30cfbe19f Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Fri, 4 Sep 2026 16:19:30 -0400 Subject: [PATCH 21/24] fix(chat): reduce favorite star motion to a half turn Use a single half rotation for favorite and unfavorite star phases while keeping the existing timing, pop, opacity, and row movement. End each visible star upright. Co-authored-by: Goose --- src/features/chat/ui/AgentModelPickerLists.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/features/chat/ui/AgentModelPickerLists.tsx b/src/features/chat/ui/AgentModelPickerLists.tsx index df4cacdc5..6145dd402 100644 --- a/src/features/chat/ui/AgentModelPickerLists.tsx +++ b/src/features/chat/ui/AgentModelPickerLists.tsx @@ -835,20 +835,20 @@ export const RecommendedModelList = forwardRef< activeStarAnimation?.phase === "out" ? { rotate: starred - ? [0, 0, -360, -360] - : [0, 0, 360, 360], + ? [0, 0, -180, -180] + : [0, 0, 180, 180], scale: [1, 0.78, 1.18, 0.9], opacity: [1, 1, 0.7, 0], } : activeStarAnimation?.phase === "moving" ? { - rotate: -360, + rotate: -180, scale: 0.9, opacity: 0, } : activeStarAnimation?.phase === "in" ? { - rotate: [-360, -180, 0, 0], + rotate: [-180, -90, 0, 0], scale: [0.9, 1.18, 0.96, 1], opacity: [0, 0, 0.7, 1], } From c1e5ffdb8ed5dc8ee45d274f9f0ef9c8575c5b46 Mon Sep 17 00:00:00 2001 From: Nick Esposito Date: Fri, 4 Sep 2026 16:41:55 -0400 Subject: [PATCH 22/24] fix(chat): wrap selected model pill around star Co-authored-by: Goose --- src/features/chat/ui/AgentModelPickerLists.tsx | 15 +++++++++------ .../chat/ui/__tests__/AgentModelPicker.test.tsx | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/features/chat/ui/AgentModelPickerLists.tsx b/src/features/chat/ui/AgentModelPickerLists.tsx index 6145dd402..fde8002d0 100644 --- a/src/features/chat/ui/AgentModelPickerLists.tsx +++ b/src/features/chat/ui/AgentModelPickerLists.tsx @@ -8,7 +8,6 @@ import { useState, } from "react"; import { - IconCheck, IconDots, IconSearch, IconStar, @@ -738,8 +737,12 @@ export const RecommendedModelList = forwardRef< rowElementsRef.current.delete(modelKey); } }} - className="flex min-w-0 items-center gap-1" + className={cn( + "flex min-w-0 items-center gap-1 rounded-sm", + isSelected && "bg-accent", + )} data-model-key={modelKey} + data-selected={isSelected || undefined} data-starred={starred || undefined} onPointerMove={(event) => { pointerPositionRef.current = { @@ -797,16 +800,16 @@ export const RecommendedModelList = forwardRef< {getModelDisplayName(model)}
- {isSelected ? ( - - ) : null} {existsInCatalog ? (