diff --git a/ui/goose2/src/app/AppShell.tsx b/ui/goose2/src/app/AppShell.tsx index 25c5324dc3..be03198ff3 100644 --- a/ui/goose2/src/app/AppShell.tsx +++ b/ui/goose2/src/app/AppShell.tsx @@ -33,11 +33,8 @@ import { loadStoredHomeSessionId } from "./lib/homeSessionStorage"; import { resolveSupportedSessionModelPreference } from "./lib/resolveSupportedSessionModelPreference"; import { useCreatePersonaNavigation } from "./hooks/useCreatePersonaNavigation"; import { AppShellContent } from "./ui/AppShellContent"; -import { acpPrepareSession, acpSetModel } from "@/shared/api/acp"; -import { - updateSessionProject, - updateSessionTitle, -} from "@/features/chat/stores/chatSessionOperations"; +import { applyLatestSessionConfig } from "@/features/chat/lib/sessionConfigRequests"; +import { updateSessionTitle } from "@/features/chat/stores/chatSessionOperations"; import { clearReplayBuffer, getAndDeleteReplayBuffer, @@ -244,14 +241,32 @@ export function AppShell({ children }: { children?: React.ReactNode }) { } const request = (async () => { + const currentProvider = () => + useAgentStore.getState().selectedProvider ?? "goose"; + + // Resolve the provider to use after an async gap. If the user changed + // their selection while we were awaiting (liveProvider differs from what + // it was before the await), prefer the live value; otherwise use the + // model-preference resolution result. + const resolveProviderAfterAwait = ( + providerAtStart: string, + sessionModelPreference: { providerId: string }, + ): string => { + const liveProvider = currentProvider(); + return liveProvider !== providerAtStart + ? liveProvider + : sessionModelPreference.providerId; + }; + if ( homeSession && !homeSession.archivedAt && homeSession.messageCount === 0 ) { + const providerAtStart = currentProvider(); const sessionModelPreference = await resolveSupportedSessionModelPreference( - selectedProvider ?? "goose", + providerAtStart, providerInventoryEntries, ); const project = homeSession.projectId @@ -260,41 +275,67 @@ export function AppShell({ children }: { children?: React.ReactNode }) { ) ?? null) : null; const workingDir = await resolveSessionCwd(project); - await acpPrepareSession( - homeSession.id, - sessionModelPreference.providerId, - workingDir, + const resolvedProviderId = resolveProviderAfterAwait( + providerAtStart, + sessionModelPreference, ); - const shouldClearHomeModel = - sessionModelPreference.providerId !== homeSession.providerId || - !sessionModelPreference.modelId; - patchSession(homeSession.id, { - providerId: sessionModelPreference.providerId, - modelId: shouldClearHomeModel ? undefined : homeSession.modelId, - modelName: shouldClearHomeModel ? undefined : homeSession.modelName, + const modelIdToApply = + resolvedProviderId === sessionModelPreference.providerId + ? sessionModelPreference.modelId + : undefined; + const result = await applyLatestSessionConfig({ + sessionId: homeSession.id, + providerId: resolvedProviderId, + workingDir, + modelId: modelIdToApply, }); - if (sessionModelPreference.modelId) { - await acpSetModel(homeSession.id, sessionModelPreference.modelId); - patchSession(homeSession.id, { - modelId: sessionModelPreference.modelId, - modelName: sessionModelPreference.modelName, - }); + if (!result.applied) { + return homeSession; } - return homeSession; + + const shouldClearHomeModel = + resolvedProviderId !== homeSession.providerId || !modelIdToApply; + patchSession(homeSession.id, { + providerId: resolvedProviderId, + modelId: + modelIdToApply ?? + (shouldClearHomeModel ? undefined : homeSession.modelId), + modelName: + modelIdToApply != null + ? sessionModelPreference.modelName + : shouldClearHomeModel + ? undefined + : homeSession.modelName, + }); + return ( + useChatSessionStore.getState().getSession(homeSession.id) ?? + homeSession + ); } + const providerAtStart = currentProvider(); const workingDir = await resolveSessionCwd(null); const sessionModelPreference = await resolveSupportedSessionModelPreference( - selectedProvider ?? "goose", + providerAtStart, providerInventoryEntries, ); + const resolvedProviderId = resolveProviderAfterAwait( + providerAtStart, + sessionModelPreference, + ); const session = await createSession({ title: DEFAULT_CHAT_TITLE, - providerId: sessionModelPreference.providerId, + providerId: resolvedProviderId, workingDir, - modelId: sessionModelPreference.modelId, - modelName: sessionModelPreference.modelName, + modelId: + resolvedProviderId === sessionModelPreference.providerId + ? sessionModelPreference.modelId + : undefined, + modelName: + resolvedProviderId === sessionModelPreference.providerId + ? sessionModelPreference.modelName + : undefined, }); setHomeSessionId(session.id); return session; @@ -309,7 +350,6 @@ export function AppShell({ children }: { children?: React.ReactNode }) { } } }, [ - selectedProvider, createSession, hasHydratedSessions, homeSession, @@ -512,14 +552,14 @@ export function AppShell({ children }: { children?: React.ReactNode }) { const handleMoveToProject = useCallback( (sessionId: string, projectId: string | null) => { + useChatSessionStore.getState().patchSession(sessionId, { projectId }); + const session = useChatSessionStore.getState().getSession(sessionId); if (!session) { return; } void (async () => { - await updateSessionProject(sessionId, projectId); - const nextProject = projectId == null ? null @@ -530,17 +570,20 @@ export function AppShell({ children }: { children?: React.ReactNode }) { if (!workingDir) { return; } - await acpPrepareSession( + await applyLatestSessionConfig({ sessionId, - session.providerId ?? selectedProvider ?? "goose", + providerId: session.providerId ?? selectedProvider ?? "goose", workingDir, - ); + modelId: session.modelId, + }); })().catch((error) => { - console.error("Failed to move chat to project:", error); - toast.error(t("notifications.moveError")); + console.error( + "Failed to update ACP session project working directory:", + error, + ); }); }, - [selectedProvider, t], + [selectedProvider], ); const handleRenameChat = useCallback( diff --git a/ui/goose2/src/app/hooks/useAppStartup.ts b/ui/goose2/src/app/hooks/useAppStartup.ts index ac15339cc1..af6533bac1 100644 --- a/ui/goose2/src/app/hooks/useAppStartup.ts +++ b/ui/goose2/src/app/hooks/useAppStartup.ts @@ -65,6 +65,7 @@ export function useAppStartup() { const applyProvidersFromInventory = ( entries: Parameters[0], + validated = false, ) => { const providers = discoverAcpProvidersFromEntries(entries); const providerAllowlist = parseProviderAllowlist( @@ -76,6 +77,7 @@ export function useAppStartup() { providerAllowlist, getModelProviders(), ), + validated, ); return providers; }; @@ -116,7 +118,7 @@ export function useAppStartup() { ...useProviderInventoryStore.getState().entries.values(), ]; if (inventoryEntries.length > 0) { - applyProvidersFromInventory(inventoryEntries); + applyProvidersFromInventory(inventoryEntries, true); } perfLog( `[perf:startup] loadProviderCatalog done in ${(performance.now() - t0).toFixed(1)}ms (n=${entries.length})`, @@ -140,7 +142,7 @@ export function useAppStartup() { inventoryStore.setEntries(entries); // Derive ACP providers from the same response - const providers = applyProvidersFromInventory(entries); + const providers = applyProvidersFromInventory(entries, true); perfLog( `[perf:startup] loadProvidersAndInventory done in ${(performance.now() - t0).toFixed(1)}ms (entries=${entries.length}, providers=${providers.length})`, diff --git a/ui/goose2/src/app/lib/resolveSupportedSessionModelPreference.test.ts b/ui/goose2/src/app/lib/resolveSupportedSessionModelPreference.test.ts index 98583b5582..da339f266b 100644 --- a/ui/goose2/src/app/lib/resolveSupportedSessionModelPreference.test.ts +++ b/ui/goose2/src/app/lib/resolveSupportedSessionModelPreference.test.ts @@ -55,4 +55,28 @@ describe("resolveSupportedSessionModelPreference", () => { providerId: "openai", }); }); + + it("preserves an exact stored provider model while inventory is unavailable", async () => { + window.localStorage.setItem( + "goose:preferredModelsByAgent", + JSON.stringify({ + "claude-acp": { + modelId: "opus", + modelName: "Claude Opus", + providerId: "claude-acp", + }, + }), + ); + mockGetProviderInventory.mockRejectedValue( + new Error("inventory unavailable"), + ); + + await expect( + resolveSupportedSessionModelPreference("claude-acp", new Map()), + ).resolves.toEqual({ + providerId: "claude-acp", + modelId: "opus", + modelName: "Claude Opus", + }); + }); }); diff --git a/ui/goose2/src/app/lib/resolveSupportedSessionModelPreference.ts b/ui/goose2/src/app/lib/resolveSupportedSessionModelPreference.ts index dedbd53ffb..84d8953a0a 100644 --- a/ui/goose2/src/app/lib/resolveSupportedSessionModelPreference.ts +++ b/ui/goose2/src/app/lib/resolveSupportedSessionModelPreference.ts @@ -5,6 +5,7 @@ import { sanitizeSessionModelPreference, type SessionModelPreference, } from "@/features/chat/lib/sessionModelPreference"; +import { getStoredModelPreference } from "@/features/chat/lib/modelPreferences"; export async function resolveSupportedSessionModelPreference( providerId: string, @@ -20,6 +21,15 @@ export async function resolveSupportedSessionModelPreference( return sessionModelPreference; } + const exactStoredPreference = preferredModel + ? null + : getStoredModelPreference(providerId); + const shouldPreserveWithoutInventory = + sessionModelPreference.providerId === providerId && + exactStoredPreference?.modelId === sessionModelPreference.modelId && + (exactStoredPreference.providerId ?? providerId) === + sessionModelPreference.providerId; + const inventoryEntry = inventoryEntries.get(sessionModelPreference.providerId) ?? (await getProviderInventory([sessionModelPreference.providerId]) @@ -27,6 +37,10 @@ export async function resolveSupportedSessionModelPreference( .catch(() => undefined)); if (!inventoryEntry) { + if (shouldPreserveWithoutInventory) { + return sessionModelPreference; + } + return { providerId: sessionModelPreference.providerId, }; diff --git a/ui/goose2/src/features/agents/stores/__tests__/agentStore.test.ts b/ui/goose2/src/features/agents/stores/__tests__/agentStore.test.ts index 5887c5877f..d11ce46f85 100644 --- a/ui/goose2/src/features/agents/stores/__tests__/agentStore.test.ts +++ b/ui/goose2/src/features/agents/stores/__tests__/agentStore.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach } from "vitest"; +import { afterEach, describe, it, expect, beforeEach } from "vitest"; import { useAgentStore } from "../agentStore"; import type { Persona, Agent } from "@/shared/types/agents"; @@ -202,3 +202,50 @@ describe("agentStore", () => { expect(custom[0].id).toBe("c"); }); }); + +describe("agentStore.setProviders", () => { + beforeEach(() => { + localStorage.clear(); + useAgentStore.setState({ + providers: [], + providersLoading: false, + selectedProvider: "claude-acp", + }); + localStorage.setItem("goose:defaultProvider", "claude-acp"); + }); + + afterEach(() => { + localStorage.clear(); + }); + + it("does not overwrite stored provider during unvalidated hydration", () => { + useAgentStore + .getState() + .setProviders([{ id: "goose", label: "Goose" }], false); + + expect(useAgentStore.getState().selectedProvider).toBe("claude-acp"); + expect(localStorage.getItem("goose:defaultProvider")).toBe("claude-acp"); + }); + + it("falls back and persists when validated and provider is missing", () => { + useAgentStore + .getState() + .setProviders([{ id: "goose", label: "Goose" }], true); + + expect(useAgentStore.getState().selectedProvider).toBe("goose"); + expect(localStorage.getItem("goose:defaultProvider")).toBe("goose"); + }); + + it("keeps valid provider during validated hydration", () => { + useAgentStore.getState().setProviders( + [ + { id: "goose", label: "Goose" }, + { id: "claude-acp", label: "Claude Code" }, + ], + true, + ); + + expect(useAgentStore.getState().selectedProvider).toBe("claude-acp"); + expect(localStorage.getItem("goose:defaultProvider")).toBe("claude-acp"); + }); +}); diff --git a/ui/goose2/src/features/agents/stores/agentStore.ts b/ui/goose2/src/features/agents/stores/agentStore.ts index e0e3eb2531..7efee9df40 100644 --- a/ui/goose2/src/features/agents/stores/agentStore.ts +++ b/ui/goose2/src/features/agents/stores/agentStore.ts @@ -75,7 +75,7 @@ interface AgentStoreActions { setAgentsLoading: (loading: boolean) => void; // Provider management - setProviders: (providers: AcpProvider[]) => void; + setProviders: (providers: AcpProvider[], validated?: boolean) => void; setProvidersLoading: (loading: boolean) => void; setSelectedProvider: (providerId: string, persist?: boolean) => void; @@ -163,10 +163,10 @@ export const useAgentStore = create((set, get) => ({ setAgentsLoading: (agentsLoading) => set({ agentsLoading }), // Provider management - setProviders: (providers) => { + setProviders: (providers, validated = true) => { const { selectedProvider } = get(); const isValid = providers.some((p) => p.id === selectedProvider); - if (!isValid && providers.length > 0) { + if (!isValid && providers.length > 0 && validated) { const fallback = providers[0].id; persistProvider(fallback); set({ providers, selectedProvider: fallback }); diff --git a/ui/goose2/src/features/chat/hooks/__tests__/useChat.compaction.test.ts b/ui/goose2/src/features/chat/hooks/__tests__/useChat.compaction.test.ts index 295c0fd69e..457783f845 100644 --- a/ui/goose2/src/features/chat/hooks/__tests__/useChat.compaction.test.ts +++ b/ui/goose2/src/features/chat/hooks/__tests__/useChat.compaction.test.ts @@ -122,6 +122,7 @@ describe("useChat compaction", () => { let preparedPersonaId: string | undefined; const ensurePrepared = vi.fn(async (personaId?: string) => { preparedPersonaId = personaId; + return undefined; }); const { result } = renderHook(() => @@ -264,4 +265,40 @@ describe("useChat compaction", () => { expect(runtime.error).toBe("prepare failed"); expect(runtime.chatState).toBe("idle"); }); + + it("does not compact when preparation is superseded", async () => { + const ensurePrepared = vi.fn().mockResolvedValue(false); + + const { result } = renderHook(() => + useChat("session-1", undefined, undefined, undefined, { + ensurePrepared, + }), + ); + + let compactResult: unknown; + await act(async () => { + compactResult = await result.current.compactConversation(); + }); + + expect(compactResult).toBe("failed"); + expect(ensurePrepared).toHaveBeenCalledWith(undefined); + expect(mockAcpSendMessage).not.toHaveBeenCalled(); + expect(mockAcpLoadSession).not.toHaveBeenCalled(); + + const messages = useChatStore.getState().messagesBySession["session-1"]; + const runtime = useChatStore.getState().getSessionRuntime("session-1"); + + expect(messages).toHaveLength(1); + expect(messages[0].content).toEqual([ + { + type: "systemNotification", + notificationType: "error", + text: "Session configuration changed while preparing. Try sending again.", + }, + ]); + expect(runtime.error).toBe( + "Session configuration changed while preparing. Try sending again.", + ); + expect(runtime.chatState).toBe("idle"); + }); }); diff --git a/ui/goose2/src/features/chat/hooks/__tests__/useChat.test.ts b/ui/goose2/src/features/chat/hooks/__tests__/useChat.test.ts index bd2d91a521..0fff7f2605 100644 --- a/ui/goose2/src/features/chat/hooks/__tests__/useChat.test.ts +++ b/ui/goose2/src/features/chat/hooks/__tests__/useChat.test.ts @@ -331,6 +331,41 @@ describe("useChat", () => { ); }); + it("does not prompt when preparation is superseded", async () => { + const ensurePrepared = vi.fn().mockResolvedValue(false); + + const { result } = renderHook(() => + useChat("session-1", undefined, undefined, undefined, { + ensurePrepared, + }), + ); + + await act(async () => { + await result.current.sendMessage("Hello"); + }); + + expect(ensurePrepared).toHaveBeenCalledTimes(1); + expect(mockAcpSendMessage).not.toHaveBeenCalled(); + + const messages = useChatStore.getState().messagesBySession["session-1"]; + const runtime = useChatStore.getState().getSessionRuntime("session-1"); + + expect(messages).toHaveLength(2); + expect(messages[0].role).toBe("user"); + expect(messages[1].content).toEqual([ + { + type: "systemNotification", + notificationType: "error", + text: "Session configuration changed while preparing. Try sending again.", + }, + ]); + expect(runtime.error).toBe( + "Session configuration changed while preparing. Try sending again.", + ); + expect(runtime.chatState).toBe("idle"); + expect(runtime.streamingMessageId).toBeNull(); + }); + it("appends an error message and removes the empty assistant placeholder when send fails", async () => { mockAcpSendMessage.mockRejectedValue( new Error("Working directory missing"), diff --git a/ui/goose2/src/features/chat/hooks/__tests__/useChatSessionController.test.ts b/ui/goose2/src/features/chat/hooks/__tests__/useChatSessionController.test.ts index 5ad52eac54..4d6c02a56c 100644 --- a/ui/goose2/src/features/chat/hooks/__tests__/useChatSessionController.test.ts +++ b/ui/goose2/src/features/chat/hooks/__tests__/useChatSessionController.test.ts @@ -4,6 +4,7 @@ import { useAgentStore } from "@/features/agents/stores/agentStore"; import { useProjectStore } from "@/features/projects/stores/projectStore"; import { useChatStore } from "../../stores/chatStore"; import { useChatSessionStore } from "../../stores/chatSessionStore"; +import { applyLatestSessionConfig } from "../../lib/sessionConfigRequests"; const mockAcpPrepareSession = vi.fn(); const mockAcpSetModel = vi.fn(); @@ -23,6 +24,16 @@ const mockPickerState = { modelStatusMessage: null as string | null, }; +function deferred() { + let resolve!: (value: T | PromiseLike) => void; + let reject!: (reason?: unknown) => void; + const promise = new Promise((promiseResolve, promiseReject) => { + resolve = promiseResolve; + reject = promiseReject; + }); + return { promise, resolve, reject }; +} + vi.mock("@/shared/api/acp", () => ({ acpPrepareSession: (...args: unknown[]) => mockAcpPrepareSession(...args), acpSetModel: (...args: unknown[]) => mockAcpSetModel(...args), @@ -360,6 +371,82 @@ describe("useChatSessionController", () => { }); }); + it("moves pending Home queued messages when preparation is superseded", async () => { + const firstPrepare = deferred(); + mockAcpPrepareSession.mockReturnValueOnce(firstPrepare.promise); + + const { result, rerender } = renderHook( + ({ sessionId }: { sessionId: string | null }) => + useChatSessionController({ sessionId }), + { + initialProps: { sessionId: null as string | null }, + }, + ); + + act(() => { + result.current.handleModelChange("claude-sonnet-4"); + useChatStore + .getState() + .enqueueMessage("__home_pending__", { text: "queued from Home" }); + }); + + useChatSessionStore.setState((state) => ({ + sessions: [ + { + id: "session-superseded-home", + title: "Chat", + providerId: "openai", + createdAt: "2026-04-21T00:00:00.000Z", + updatedAt: "2026-04-21T00:00:00.000Z", + messageCount: 0, + }, + ...state.sessions, + ], + })); + + rerender({ sessionId: "session-superseded-home" }); + + await waitFor(() => { + expect(mockAcpPrepareSession).toHaveBeenCalledWith( + "session-superseded-home", + "anthropic", + "/tmp/project", + ); + }); + + const latestConfig = applyLatestSessionConfig({ + sessionId: "session-superseded-home", + providerId: "anthropic", + workingDir: "/tmp/other-project", + modelId: "claude-sonnet-4", + }); + + firstPrepare.resolve(); + + await waitFor(() => { + expect(mockAcpPrepareSession).toHaveBeenCalledWith( + "session-superseded-home", + "anthropic", + "/tmp/other-project", + ); + }); + await expect(latestConfig).resolves.toEqual({ applied: true }); + + await waitFor(() => { + expect( + useChatStore.getState().queuedMessageBySession[ + "session-superseded-home" + ], + ).toEqual({ text: "queued from Home" }); + }); + expect( + useChatStore.getState().queuedMessageBySession.__home_pending__, + ).toBeUndefined(); + expect( + window.localStorage.getItem("goose:preferredModelsByAgent"), + ).toBeNull(); + }); + it("does not persist or record a pending Home model when ACP rejects it", async () => { mockAcpSetModel.mockRejectedValueOnce(new Error("set model failed")); diff --git a/ui/goose2/src/features/chat/hooks/__tests__/useResolvedAgentModelPicker.test.ts b/ui/goose2/src/features/chat/hooks/__tests__/useResolvedAgentModelPicker.test.ts index e84f58465f..c4d3c6314f 100644 --- a/ui/goose2/src/features/chat/hooks/__tests__/useResolvedAgentModelPicker.test.ts +++ b/ui/goose2/src/features/chat/hooks/__tests__/useResolvedAgentModelPicker.test.ts @@ -565,7 +565,7 @@ describe("useResolvedAgentModelPicker", () => { setPendingProviderId: vi.fn(), setPendingModelSelection: vi.fn(), setGlobalSelectedProvider: vi.fn(), - prepareSelectedProvider: vi.fn(), + prepareSelectedProvider: vi.fn().mockResolvedValue(true), }), ); @@ -664,7 +664,7 @@ describe("useResolvedAgentModelPicker", () => { setPendingProviderId: vi.fn(), setPendingModelSelection: vi.fn(), setGlobalSelectedProvider: vi.fn(), - prepareSelectedProvider: vi.fn(), + prepareSelectedProvider: vi.fn().mockResolvedValue(true), }), ); @@ -688,4 +688,198 @@ describe("useResolvedAgentModelPicker", () => { }); }); }); + + it("does not persist a superseded explicit model selection", async () => { + const prepareSelectedProvider = vi.fn().mockResolvedValue(false); + + mockUseAgentModelPickerState.mockImplementation( + ({ + onModelSelected, + }: { + onModelSelected?: (model: { + id: string; + name: string; + displayName?: string; + providerId?: string; + }) => void; + }) => ({ + pickerAgents: [{ id: "goose", label: "Goose" }], + availableModels: [ + { + id: "gpt-5.4", + name: "GPT-5.4", + displayName: "GPT-5.4", + providerId: "openai", + }, + ], + modelsLoading: false, + modelStatusMessage: null, + handleProviderChange: vi.fn(), + handleModelChange: (modelId: string) => + onModelSelected?.({ + id: modelId, + name: "GPT-5.4", + displayName: "GPT-5.4", + providerId: "openai", + }), + }), + ); + + const { result } = renderHook(() => + useResolvedAgentModelPicker({ + providers: [ + { id: "goose", label: "Goose" }, + { id: "openai", label: "OpenAI" }, + ], + selectedProvider: "openai", + sessionId: "session-1", + session: { + id: "session-1", + title: "Chat", + providerId: "openai", + modelId: "current", + modelName: "current", + createdAt: "2026-04-21T00:00:00.000Z", + updatedAt: "2026-04-21T00:00:00.000Z", + messageCount: 0, + }, + pendingModelSelection: undefined, + setPendingProviderId: vi.fn(), + setPendingModelSelection: vi.fn(), + setGlobalSelectedProvider: vi.fn(), + prepareSelectedProvider, + }), + ); + + act(() => { + result.current.handleModelChange("gpt-5.4"); + }); + + await waitFor(() => { + expect(prepareSelectedProvider).toHaveBeenCalledWith("openai", { + id: "gpt-5.4", + name: "GPT-5.4", + providerId: "openai", + source: "explicit", + }); + }); + expect(localStorage.getItem("goose:preferredModelsByAgent")).toBeNull(); + }); + + it("preserves persisted Claude Code / Opus during empty inventory and catalog", () => { + useProviderCatalogStore.getState().reset(); + + window.localStorage.setItem( + "goose:preferredModelsByAgent", + JSON.stringify({ + "claude-acp": { + modelId: "opus", + modelName: "Claude Opus", + providerId: "claude-acp", + }, + }), + ); + + mockUseProviderInventory.mockReturnValue({ + getEntry: () => undefined, + }); + + mockUseAgentModelPickerState.mockImplementation(() => ({ + pickerAgents: [{ id: "goose", label: "Goose" }], + availableModels: [], + modelsLoading: true, + modelStatusMessage: null, + handleProviderChange: vi.fn(), + handleModelChange: vi.fn(), + })); + + const { result } = renderHook(() => + useResolvedAgentModelPicker({ + providers: [], + selectedProvider: "claude-acp", + sessionId: null, + session: undefined, + pendingModelSelection: undefined, + setPendingProviderId: vi.fn(), + setPendingModelSelection: vi.fn(), + setGlobalSelectedProvider: vi.fn(), + prepareSelectedProvider: vi.fn(), + }), + ); + + expect(result.current.selectedAgentId).toBe("claude-acp"); + expect(result.current.effectiveModelSelection).toEqual({ + id: "opus", + name: "Claude Opus", + providerId: "claude-acp", + source: "explicit", + }); + }); + + it("retains selection after validated inventory confirms the agent", () => { + window.localStorage.setItem( + "goose:preferredModelsByAgent", + JSON.stringify({ + "claude-acp": { + modelId: "opus", + modelName: "Claude Opus", + providerId: "claude-acp", + }, + }), + ); + + mockUseProviderInventory.mockReturnValue({ + getEntry: (id: string) => + id === "claude-acp" + ? { + providerId: "claude-acp", + category: "agent", + models: [ + { id: "opus", name: "Claude Opus", recommended: false }, + { id: "sonnet", name: "Claude Sonnet", recommended: true }, + ], + } + : undefined, + }); + + mockUseAgentModelPickerState.mockImplementation(() => ({ + pickerAgents: [ + { id: "goose", label: "Goose" }, + { id: "claude-acp", label: "Claude Code" }, + ], + availableModels: [ + { id: "opus", name: "Claude Opus", providerId: "claude-acp" }, + { id: "sonnet", name: "Claude Sonnet", providerId: "claude-acp" }, + ], + modelsLoading: false, + modelStatusMessage: null, + handleProviderChange: vi.fn(), + handleModelChange: vi.fn(), + })); + + const { result } = renderHook(() => + useResolvedAgentModelPicker({ + providers: [ + { id: "goose", label: "Goose" }, + { id: "claude-acp", label: "Claude Code" }, + ], + selectedProvider: "claude-acp", + sessionId: null, + session: undefined, + pendingModelSelection: undefined, + setPendingProviderId: vi.fn(), + setPendingModelSelection: vi.fn(), + setGlobalSelectedProvider: vi.fn(), + prepareSelectedProvider: vi.fn(), + }), + ); + + expect(result.current.selectedAgentId).toBe("claude-acp"); + expect(result.current.effectiveModelSelection).toEqual({ + id: "opus", + name: "Claude Opus", + providerId: "claude-acp", + source: "explicit", + }); + }); }); diff --git a/ui/goose2/src/features/chat/hooks/useChat.ts b/ui/goose2/src/features/chat/hooks/useChat.ts index a35bdb8759..ca16fa3402 100644 --- a/ui/goose2/src/features/chat/hooks/useChat.ts +++ b/ui/goose2/src/features/chat/hooks/useChat.ts @@ -34,6 +34,7 @@ import type { ChatSendOptions } from "../types"; const MANUAL_COMPACT_TRIGGER = "/compact"; const EMPTY_MESSAGES: Message[] = []; type CompactConversationResult = "completed" | "failed" | "skipped"; +type EnsurePrepared = (personaId?: string) => Promise; function createCompactionConfirmationMessage() { return createSystemNotificationMessage( @@ -66,6 +67,16 @@ function getErrorMessage(error: unknown): string { return "Unknown error"; } +async function ensurePreparedForPrompt( + ensurePrepared: EnsurePrepared | undefined, + personaId?: string, +) { + const prepared = await ensurePrepared?.(personaId); + if (prepared === false) { + throw new Error(i18n.t("chat:errors.sessionPreparationSuperseded")); + } +} + function markMessageStopped(sessionId: string, messageId: string) { useChatStore.getState().updateMessage(sessionId, messageId, (message) => { if ( @@ -102,7 +113,7 @@ export function useChat( personaInfo?: { id: string; name: string }, options?: { onMessageAccepted?: (sessionId: string) => void; - ensurePrepared?: (personaId?: string) => Promise; + ensurePrepared?: EnsurePrepared; }, ) { const abortRef = useRef(null); @@ -243,7 +254,10 @@ export function useChat( abortRef.current = abort; try { - await options?.ensurePrepared?.(effectivePersonaInfo?.id); + await ensurePreparedForPrompt( + options?.ensurePrepared, + effectivePersonaInfo?.id, + ); setChatState(sessionId, "streaming"); const promptWithPaths = appendAttachmentPaths(text.trim(), attachments); @@ -386,7 +400,10 @@ export function useChat( setError(sessionId, null); try { - await options?.ensurePrepared?.(effectivePersonaInfo?.id); + await ensurePreparedForPrompt( + options?.ensurePrepared, + effectivePersonaInfo?.id, + ); } catch (err) { const errorMessage = getErrorMessage(err); addMessage( diff --git a/ui/goose2/src/features/chat/hooks/useChatInputSubmit.ts b/ui/goose2/src/features/chat/hooks/useChatInputSubmit.ts index c6e0595459..3c53273732 100644 --- a/ui/goose2/src/features/chat/hooks/useChatInputSubmit.ts +++ b/ui/goose2/src/features/chat/hooks/useChatInputSubmit.ts @@ -3,13 +3,13 @@ import type { SkillCommandMatch } from "@/features/skills/lib/skillChatPrompt"; import type { ChatAttachmentDraft } from "@/shared/types/messages"; import { skillDraftSnapshotsMatch } from "../lib/chatInputSnapshots"; import { submitComposerMessage } from "../lib/submitComposerMessage"; -import type { ChatInputProps, ChatSkillDraft } from "../types"; +import type { ChatInputSendHandler, ChatSkillDraft } from "../types"; interface UseChatInputSubmitOptions { attachmentsRef: RefObject; selectedSkillsRef: RefObject; selectedPersonaId?: string | null; - onSend: ChatInputProps["onSend"]; + onSend: ChatInputSendHandler; setSelectedSkills: (skills: ChatSkillDraft[]) => void; resolveSkillSlashCommand: ( message: string, diff --git a/ui/goose2/src/features/chat/hooks/useChatSessionController.ts b/ui/goose2/src/features/chat/hooks/useChatSessionController.ts index 041b7cf736..1212ebc281 100644 --- a/ui/goose2/src/features/chat/hooks/useChatSessionController.ts +++ b/ui/goose2/src/features/chat/hooks/useChatSessionController.ts @@ -20,14 +20,13 @@ import { resolveProjectDefaultArtifactRoot, } from "@/features/projects/lib/chatProjectContext"; import { setStoredModelPreference } from "../lib/modelPreferences"; +import { applyLatestSessionConfig } from "../lib/sessionConfigRequests"; import { shouldAutoCompactContext, supportsContextAutoCompaction, supportsContextCompactionControls, } from "../lib/autoCompact"; import { resolveSessionCwd } from "@/features/projects/lib/sessionCwdSelection"; -import { acpPrepareSession, acpSetModel } from "@/shared/api/acp"; -import { updateSessionProject } from "../stores/chatSessionOperations"; import { useResolvedAgentModelPicker, type PreferredModelSelection, @@ -42,6 +41,15 @@ interface UseChatSessionControllerOptions { const PENDING_HOME_SESSION_ID = "__home_pending__"; const EMPTY_SKILL_DRAFTS: ChatSkillDraft[] = []; +function movePendingHomeQueuedMessage(sessionId: string) { + const chatState = useChatStore.getState(); + const pendingQueue = + chatState.queuedMessageBySession[PENDING_HOME_SESSION_ID] ?? null; + if (pendingQueue && !chatState.queuedMessageBySession[sessionId]) { + chatState.enqueueMessage(sessionId, pendingQueue); + } +} + export function useChatSessionController({ sessionId, onMessageAccepted, @@ -165,15 +173,20 @@ export function useChatSessionController({ modelSelection?: PreferredModelSelection | null, ) => { if (!sessionId) { - return; + return false; } const workingDir = await resolveSessionCwd( nextProject, nextWorkspacePath, ); - await acpPrepareSession(sessionId, providerId, workingDir); - if (!modelSelection?.id) { - return; + const result = await applyLatestSessionConfig({ + sessionId, + providerId, + workingDir, + modelId: modelSelection?.id, + }); + if (!result.applied || !modelSelection?.id) { + return result.applied; } const sessionStore = useChatSessionStore.getState(); @@ -183,14 +196,14 @@ export function useChatSessionController({ liveSession?.modelName === modelSelection.name; if (modelAlreadyApplied) { - return; + return true; } - await acpSetModel(sessionId, modelSelection.id); sessionStore.patchSession(sessionId, { modelId: modelSelection.id, modelName: modelSelection.name, }); + return true; }, [activeWorkspace?.path, project, sessionId], ); @@ -318,18 +331,16 @@ export function useChatSessionController({ .projects.find((candidate) => candidate.id === projectId) ?? null); - void (async () => { - await updateSessionProject(sessionId, projectId); - if (!selectedProvider) { - return; - } - await prepareCurrentSession( - selectedProvider, - nextProject, - activeWorkspace?.path, - effectiveModelSelection, - ); - })().catch((error) => { + useChatSessionStore.getState().patchSession(sessionId, { projectId }); + if (!selectedProvider) { + return; + } + void prepareCurrentSession( + selectedProvider, + nextProject, + activeWorkspace?.path, + effectiveModelSelection, + ).catch((error) => { console.error("Failed to update ACP session working directory:", error); }); }, @@ -431,6 +442,7 @@ export function useChatSessionController({ selectedProvider, project, activeWorkspace?.path, + effectiveModelSelection, ) : undefined, }, @@ -710,6 +722,7 @@ export function useChatSessionController({ personaId?: string | undefined; modelId?: string | undefined; modelName?: string | undefined; + projectId?: string | null; } = {}; if (hasPendingProvider) { @@ -720,15 +733,14 @@ export function useChatSessionController({ if (hasPendingPersona) { patch.personaId = nextPersonaId; } - if (Object.keys(patch).length > 0) { - useChatSessionStore.getState().patchSession(sessionId, patch); + if (hasPendingProject) { + patch.projectId = nextProjectId ?? null; } + useChatSessionStore.getState().patchSession(sessionId, patch); + try { - if (hasPendingProject) { - await updateSessionProject(sessionId, nextProjectId ?? null); - } - await prepareCurrentSession( + const applied = await prepareCurrentSession( nextProviderId, nextProject, activeWorkspace?.path, @@ -737,7 +749,7 @@ export function useChatSessionController({ if (cancelled) { return; } - if (pendingModelSelection?.source === "explicit") { + if (applied && pendingModelSelection?.source === "explicit") { const agentId = resolveAgentProviderCatalogIdStrictFromEntries( catalogEntries, @@ -760,16 +772,7 @@ export function useChatSessionController({ setPendingModelSelection(undefined); } - const latestChatState = useChatStore.getState(); - const latestPendingQueue = - latestChatState.queuedMessageBySession[PENDING_HOME_SESSION_ID] ?? null; - if ( - latestPendingQueue && - !latestChatState.queuedMessageBySession[sessionId] - ) { - latestChatState.enqueueMessage(sessionId, latestPendingQueue); - } - + movePendingHomeQueuedMessage(sessionId); useChatStore.getState().clearDraft(PENDING_HOME_SESSION_ID); useChatStore.getState().clearSkillDrafts(PENDING_HOME_SESSION_ID); useChatStore.getState().dismissQueuedMessage(PENDING_HOME_SESSION_ID); diff --git a/ui/goose2/src/features/chat/hooks/useResolvedAgentModelPicker.ts b/ui/goose2/src/features/chat/hooks/useResolvedAgentModelPicker.ts index 180b75cd44..85da695524 100644 --- a/ui/goose2/src/features/chat/hooks/useResolvedAgentModelPicker.ts +++ b/ui/goose2/src/features/chat/hooks/useResolvedAgentModelPicker.ts @@ -1,10 +1,9 @@ -import { useEffect, useMemo, useState } from "react"; +import { useEffect, useMemo, useRef, useState } from "react"; import type { AcpProvider } from "@/shared/api/acp"; import { useProviderInventory } from "@/features/providers/hooks/useProviderInventory"; import { resolveAgentProviderCatalogIdStrictFromEntries } from "@/features/providers/providerCatalog"; import { useProviderCatalogStore } from "@/features/providers/stores/providerCatalogStore"; import { getClient } from "@/shared/api/acpConnection"; -import { acpSetModel } from "@/shared/api/acp"; import { useChatSessionStore, type ChatSession, @@ -40,7 +39,7 @@ interface UseResolvedAgentModelPickerOptions { prepareSelectedProvider: ( providerId: string, modelSelection?: PreferredModelSelection | null, - ) => Promise; + ) => Promise; } function isModelAlias(modelId?: string | null): boolean { @@ -61,6 +60,12 @@ export function useResolvedAgentModelPicker({ const catalogEntries = useProviderCatalogStore((state) => state.entries); const catalogLoaded = useProviderCatalogStore((state) => state.loaded); const { getEntry: getProviderInventoryEntry } = useProviderInventory(); + // Monotonic version counter shared across onProviderSelected and + // onModelSelected. Any user interaction (provider OR model change) bumps + // this, which invalidates in-flight async work from either callback — + // intentionally cross-callback so a rapid provider switch also cancels a + // stale model mutation and vice versa. + const selectionVersionRef = useRef(0); const [gooseDefaultSelection, setGooseDefaultSelection] = useState(null); @@ -212,6 +217,8 @@ export function useResolvedAgentModelPicker({ providers, selectedProvider, onProviderSelected: (providerId) => { + selectionVersionRef.current += 1; + const versionAtSelection = selectionVersionRef.current; const requestedAgentId = resolveAgentProviderCatalogIdStrictFromEntries( catalogEntries, providerId, @@ -259,6 +266,9 @@ export function useResolvedAgentModelPicker({ setGlobalSelectedProvider(nextProviderId); void prepareSelectedProvider(nextProviderId, nextModelSelection).catch( (error) => { + if (selectionVersionRef.current !== versionAtSelection) { + return; + } console.error("Failed to update ACP session provider:", error); }, ); @@ -267,6 +277,12 @@ export function useResolvedAgentModelPicker({ const modelId = model.id; const modelName = model.displayName ?? model.name ?? model.id; const nextProviderId = model.providerId ?? selectedProvider; + const nextModelSelection: PreferredModelSelection = { + id: modelId, + name: modelName, + providerId: nextProviderId, + source: "explicit", + }; const nextStoredModelPreference = { modelId, modelName, @@ -278,15 +294,14 @@ export function useResolvedAgentModelPicker({ setPendingProviderId(nextProviderId); setGlobalSelectedProvider(nextProviderId); } - setPendingModelSelection({ - id: modelId, - name: modelName, - providerId: nextProviderId, - source: "explicit", - }); + setPendingModelSelection(nextModelSelection); return; } + // No-op guard: if the selected model/provider already matches the + // session, bail out without bumping the version counter. Bumping + // before this check would invalidate in-flight async work from the + // original selection that is still correctly configuring the backend. if ( !session || (modelId === session.modelId && @@ -295,6 +310,9 @@ export function useResolvedAgentModelPicker({ return; } + selectionVersionRef.current += 1; + const versionAtSelection = selectionVersionRef.current; + const previousStoredModelPreference = getStoredModelPreference(selectedAgentId); const previousProviderId = session.providerId; @@ -317,12 +335,18 @@ export function useResolvedAgentModelPicker({ void (async () => { try { - if (providerChanged && nextProviderId) { - await prepareSelectedProvider(nextProviderId); + const applied = await prepareSelectedProvider( + nextProviderId, + nextModelSelection, + ); + if (!applied || selectionVersionRef.current !== versionAtSelection) { + return; } - await acpSetModel(sessionId, modelId); setStoredModelPreference(selectedAgentId, nextStoredModelPreference); } catch (error) { + if (selectionVersionRef.current !== versionAtSelection) { + return; + } console.error("Failed to set model:", error); if (providerChanged && previousProviderId) { setGlobalSelectedProvider(previousProviderId); @@ -342,11 +366,18 @@ export function useResolvedAgentModelPicker({ }); void (async () => { try { - if (providerChanged && previousProviderId) { - await prepareSelectedProvider(previousProviderId); - } - if (previousModelId) { - await acpSetModel(sessionId, previousModelId); + if (previousProviderId) { + await prepareSelectedProvider( + previousProviderId, + previousModelId + ? { + id: previousModelId, + name: previousModelName ?? previousModelId, + providerId: previousProviderId, + source: "explicit", + } + : null, + ); } } catch (rollbackError) { console.error( diff --git a/ui/goose2/src/features/chat/lib/__tests__/agentProviderResolution.test.ts b/ui/goose2/src/features/chat/lib/__tests__/agentProviderResolution.test.ts new file mode 100644 index 0000000000..834ed4b0bb --- /dev/null +++ b/ui/goose2/src/features/chat/lib/__tests__/agentProviderResolution.test.ts @@ -0,0 +1,148 @@ +import { describe, expect, it } from "vitest"; +import { resolveSelectedAgentId } from "../agentProviderResolution"; +import type { ProviderCatalogEntry } from "@/shared/types/providers"; + +const catalogEntries: ProviderCatalogEntry[] = [ + { + id: "claude-acp", + displayName: "Claude Code", + category: "agent", + description: "Claude Code", + setupMethod: "cli_auth", + group: "default", + aliases: ["claude-acp", "claude_code", "claude"], + }, + { + id: "openai", + displayName: "OpenAI", + category: "model", + description: "OpenAI", + setupMethod: "single_api_key", + group: "default", + }, +]; + +const noInventory = () => undefined; + +describe("resolveSelectedAgentId", () => { + it("returns goose when no provider is selected", () => { + expect( + resolveSelectedAgentId({ + catalogEntries, + catalogLoaded: true, + selectedProvider: undefined, + getProviderInventoryEntry: noInventory, + }), + ).toBe("goose"); + }); + + it("resolves known agent from catalog", () => { + expect( + resolveSelectedAgentId({ + catalogEntries, + catalogLoaded: true, + selectedProvider: "claude-acp", + getProviderInventoryEntry: noInventory, + }), + ).toBe("claude-acp"); + }); + + it("returns goose for model providers with catalog loaded", () => { + expect( + resolveSelectedAgentId({ + catalogEntries, + catalogLoaded: true, + selectedProvider: "openai", + getProviderInventoryEntry: noInventory, + }), + ).toBe("goose"); + }); + + it("preserves persisted claude-acp during empty inventory before catalog loads", () => { + expect( + resolveSelectedAgentId({ + catalogEntries: [], + catalogLoaded: false, + selectedProvider: "claude-acp", + getProviderInventoryEntry: noInventory, + }), + ).toBe("claude-acp"); + }); + + it("preserves unknown provider before catalog loads when inventory is empty", () => { + expect( + resolveSelectedAgentId({ + catalogEntries: [], + catalogLoaded: false, + selectedProvider: "some-future-agent", + getProviderInventoryEntry: noInventory, + }), + ).toBe("some-future-agent"); + }); + + it("falls back to goose for model provider identified by inventory before catalog", () => { + const getEntry = (id: string) => + id === "openai" + ? ({ + providerId: "openai", + category: "model" as const, + configured: true, + refreshing: false, + models: [], + } as never) + : undefined; + + expect( + resolveSelectedAgentId({ + catalogEntries: [], + catalogLoaded: false, + selectedProvider: "openai", + getProviderInventoryEntry: getEntry, + }), + ).toBe("goose"); + }); + + it("preserves agent provider identified by inventory before catalog", () => { + const getEntry = (id: string) => + id === "claude-acp" + ? ({ + providerId: "claude-acp", + category: "agent" as const, + configured: true, + refreshing: false, + models: [], + } as never) + : undefined; + + expect( + resolveSelectedAgentId({ + catalogEntries: [], + catalogLoaded: false, + selectedProvider: "claude-acp", + getProviderInventoryEntry: getEntry, + }), + ).toBe("claude-acp"); + }); + + it("falls back to goose after catalog validates provider as non-agent", () => { + expect( + resolveSelectedAgentId({ + catalogEntries, + catalogLoaded: true, + selectedProvider: "openai", + getProviderInventoryEntry: noInventory, + }), + ).toBe("goose"); + }); + + it("falls back to goose after catalog validates unknown provider", () => { + expect( + resolveSelectedAgentId({ + catalogEntries, + catalogLoaded: true, + selectedProvider: "nonexistent-provider", + getProviderInventoryEntry: noInventory, + }), + ).toBe("goose"); + }); +}); diff --git a/ui/goose2/src/features/chat/lib/agentProviderResolution.ts b/ui/goose2/src/features/chat/lib/agentProviderResolution.ts index 95aca1eebe..fc13652f41 100644 --- a/ui/goose2/src/features/chat/lib/agentProviderResolution.ts +++ b/ui/goose2/src/features/chat/lib/agentProviderResolution.ts @@ -34,6 +34,13 @@ export function resolveSelectedAgentId({ if (inventoryEntry?.category === "agent") { return selectedProvider; } + // Catalog not loaded and no inventory info — preserve the stored + // selection so the UI doesn't briefly flash "Goose" before validation + // completes. Fall back to "goose" only when there is no selection or + // after the catalog has loaded and proven the provider is not an agent. + if (!inventoryEntry) { + return selectedProvider; + } } return "goose"; diff --git a/ui/goose2/src/features/chat/lib/modelDisplayLabel.ts b/ui/goose2/src/features/chat/lib/modelDisplayLabel.ts new file mode 100644 index 0000000000..e1bf7c283c --- /dev/null +++ b/ui/goose2/src/features/chat/lib/modelDisplayLabel.ts @@ -0,0 +1,88 @@ +import type { ModelOption } from "../types"; + +interface ModelDisplayLabelOptions { + currentModelId?: string | null; + currentModelName?: string | null; + currentModelProviderId?: string | null; + availableModels?: ModelOption[]; +} + +interface PickerTriggerLabelOptions extends ModelDisplayLabelOptions { + selectedAgentLabel?: string | null; +} + +function normalizeLabel(label?: string | null) { + const trimmed = label?.trim(); + return trimmed ? trimmed : null; +} + +function getModelDisplayName(model: ModelOption) { + return normalizeLabel(model.displayName) ?? normalizeLabel(model.name); +} + +function findSelectedInventoryModel({ + currentModelId, + currentModelProviderId, + availableModels = [], +}: Pick< + ModelDisplayLabelOptions, + "currentModelId" | "currentModelProviderId" | "availableModels" +>) { + const selectedModelId = normalizeLabel(currentModelId); + if (!selectedModelId) { + return null; + } + + const matches = availableModels.filter( + (model) => model.id === selectedModelId, + ); + if (matches.length === 0) { + return null; + } + + if (currentModelProviderId) { + return ( + matches.find((model) => model.providerId === currentModelProviderId) ?? + matches.find((model) => !model.providerId) ?? + null + ); + } + + return matches[0] ?? null; +} + +export function resolveDisplayModelLabel({ + currentModelId, + currentModelName, + currentModelProviderId, + availableModels = [], +}: ModelDisplayLabelOptions) { + const inventoryModel = findSelectedInventoryModel({ + currentModelId, + currentModelProviderId, + availableModels, + }); + const inventoryLabel = inventoryModel + ? getModelDisplayName(inventoryModel) + : null; + if (inventoryLabel) { + return inventoryLabel; + } + + const selectedModelId = normalizeLabel(currentModelId); + const modelName = normalizeLabel(currentModelName); + if (modelName && modelName !== selectedModelId) { + return modelName; + } + + return null; +} + +export function resolvePickerTriggerLabel({ + selectedAgentLabel, + ...modelOptions +}: PickerTriggerLabelOptions) { + return ( + resolveDisplayModelLabel(modelOptions) ?? normalizeLabel(selectedAgentLabel) + ); +} diff --git a/ui/goose2/src/features/chat/lib/modelPreferences.ts b/ui/goose2/src/features/chat/lib/modelPreferences.ts index 7055deb894..10230bfb80 100644 --- a/ui/goose2/src/features/chat/lib/modelPreferences.ts +++ b/ui/goose2/src/features/chat/lib/modelPreferences.ts @@ -63,6 +63,11 @@ export function getStoredModelPreference( export function getStoredModelPreferenceForProvider( providerId: string, ): StoredModelPreference | null { + const exactPreference = getStoredModelPreference(providerId); + if (exactPreference) { + return exactPreference; + } + const agentId = resolveAgentProviderCatalogIdStrict(providerId) ?? "goose"; return getStoredModelPreference(agentId); } diff --git a/ui/goose2/src/features/chat/lib/sessionConfigRequests.test.ts b/ui/goose2/src/features/chat/lib/sessionConfigRequests.test.ts new file mode 100644 index 0000000000..11587b4f56 --- /dev/null +++ b/ui/goose2/src/features/chat/lib/sessionConfigRequests.test.ts @@ -0,0 +1,234 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { applyLatestSessionConfig } from "./sessionConfigRequests"; + +const mockAcpPrepareSession = vi.fn(); +const mockAcpSetModel = vi.fn(); + +vi.mock("@/shared/api/acp", () => ({ + acpPrepareSession: (...args: unknown[]) => mockAcpPrepareSession(...args), + acpSetModel: (...args: unknown[]) => mockAcpSetModel(...args), +})); + +function deferred() { + let resolve!: (value: T | PromiseLike) => void; + let reject!: (reason?: unknown) => void; + const promise = new Promise((promiseResolve, promiseReject) => { + resolve = promiseResolve; + reject = promiseReject; + }); + return { promise, resolve, reject }; +} + +describe("applyLatestSessionConfig", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockAcpPrepareSession.mockResolvedValue(undefined); + mockAcpSetModel.mockResolvedValue(undefined); + }); + + it("replays the latest provider and model after a stale request finishes", async () => { + const oldPrepare = deferred(); + const oldSetModel = deferred(); + const newPrepare = deferred(); + const newSetModel = deferred(); + + mockAcpPrepareSession.mockImplementation( + (_sessionId: string, providerId: string) => + providerId === "old-provider" ? oldPrepare.promise : newPrepare.promise, + ); + mockAcpSetModel.mockImplementation((_sessionId: string, modelId: string) => + modelId === "old-model" ? oldSetModel.promise : newSetModel.promise, + ); + + const oldResult = applyLatestSessionConfig({ + sessionId: "session-latest", + providerId: "old-provider", + workingDir: "/old", + modelId: "old-model", + }); + const newResult = applyLatestSessionConfig({ + sessionId: "session-latest", + providerId: "new-provider", + workingDir: "/new", + modelId: "new-model", + }); + + await vi.waitFor(() => { + expect(mockAcpPrepareSession).toHaveBeenCalledWith( + "session-latest", + "old-provider", + "/old", + ); + }); + + oldPrepare.resolve(); + await vi.waitFor(() => { + expect(mockAcpSetModel).toHaveBeenCalledWith( + "session-latest", + "old-model", + ); + }); + + oldSetModel.resolve(); + await vi.waitFor(() => { + expect(mockAcpPrepareSession).toHaveBeenCalledWith( + "session-latest", + "new-provider", + "/new", + ); + }); + + newPrepare.resolve(); + await vi.waitFor(() => { + expect(mockAcpSetModel).toHaveBeenCalledWith( + "session-latest", + "new-model", + ); + }); + + newSetModel.resolve(); + + await expect(oldResult).resolves.toEqual({ applied: false }); + await expect(newResult).resolves.toEqual({ applied: true }); + }); + + it("continues to the latest request when a stale request fails", async () => { + const oldPrepare = deferred(); + const newPrepare = deferred(); + const newSetModel = deferred(); + + mockAcpPrepareSession.mockImplementation( + (_sessionId: string, providerId: string) => + providerId === "old-provider" ? oldPrepare.promise : newPrepare.promise, + ); + mockAcpSetModel.mockReturnValue(newSetModel.promise); + + const oldResult = applyLatestSessionConfig({ + sessionId: "session-stale-failure", + providerId: "old-provider", + workingDir: "/old", + modelId: "old-model", + }); + const newResult = applyLatestSessionConfig({ + sessionId: "session-stale-failure", + providerId: "new-provider", + workingDir: "/new", + modelId: "new-model", + }); + + oldPrepare.reject(new Error("old prepare failed")); + await vi.waitFor(() => { + expect(mockAcpPrepareSession).toHaveBeenCalledWith( + "session-stale-failure", + "new-provider", + "/new", + ); + }); + + newPrepare.resolve(); + await vi.waitFor(() => { + expect(mockAcpSetModel).toHaveBeenCalledWith( + "session-stale-failure", + "new-model", + ); + }); + newSetModel.resolve(); + + await expect(oldResult).resolves.toEqual({ applied: false }); + await expect(newResult).resolves.toEqual({ applied: true }); + }); + + it("treats superseded requests as applied when the full session config matches", async () => { + const firstPrepare = deferred(); + const firstSetModel = deferred(); + + // First call gets deferred promises; subsequent calls resolve immediately + mockAcpPrepareSession.mockReturnValueOnce(firstPrepare.promise); + mockAcpSetModel.mockReturnValueOnce(firstSetModel.promise); + + // Both requests have identical config (provider, workingDir, model) + const oldResult = applyLatestSessionConfig({ + sessionId: "session-same-config", + providerId: "openai", + workingDir: "/project", + modelId: "gpt-5.4", + }); + const newResult = applyLatestSessionConfig({ + sessionId: "session-same-config", + providerId: "openai", + workingDir: "/project", + modelId: "gpt-5.4", + }); + + // The queue executes the first request's prepare (stale) + await vi.waitFor(() => { + expect(mockAcpPrepareSession).toHaveBeenCalledWith( + "session-same-config", + "openai", + "/project", + ); + }); + firstPrepare.resolve(); + + // Then the first request's setModel + await vi.waitFor(() => { + expect(mockAcpSetModel).toHaveBeenCalledWith( + "session-same-config", + "gpt-5.4", + ); + }); + firstSetModel.resolve(); + + // After the stale request finishes, the queue replays the latest + // (which has the same config and resolves immediately via default mock) + // Both resolve as applied since the final config matches both requests + await expect(oldResult).resolves.toEqual({ applied: true }); + await expect(newResult).resolves.toEqual({ applied: true }); + }); + + it("treats superseded requests as not applied when workingDir differs", async () => { + const firstPrepare = deferred(); + const firstSetModel = deferred(); + + // First call gets deferred promises; subsequent calls resolve immediately + mockAcpPrepareSession.mockReturnValueOnce(firstPrepare.promise); + mockAcpSetModel.mockReturnValueOnce(firstSetModel.promise); + + const oldResult = applyLatestSessionConfig({ + sessionId: "session-diff-dir", + providerId: "openai", + workingDir: "/old", + modelId: "gpt-5.4", + }); + const newResult = applyLatestSessionConfig({ + sessionId: "session-diff-dir", + providerId: "openai", + workingDir: "/new", + modelId: "gpt-5.4", + }); + + // First request executes (stale) with workingDir "/old" + await vi.waitFor(() => { + expect(mockAcpPrepareSession).toHaveBeenCalledWith( + "session-diff-dir", + "openai", + "/old", + ); + }); + firstPrepare.resolve(); + + await vi.waitFor(() => { + expect(mockAcpSetModel).toHaveBeenCalledWith( + "session-diff-dir", + "gpt-5.4", + ); + }); + firstSetModel.resolve(); + + // After the stale request finishes, the queue replays the latest + // with workingDir "/new" (resolves immediately via default mock) + // Old request is not applied (workingDir differs), new one is applied + await expect(oldResult).resolves.toEqual({ applied: false }); + await expect(newResult).resolves.toEqual({ applied: true }); + }); +}); diff --git a/ui/goose2/src/features/chat/lib/sessionConfigRequests.ts b/ui/goose2/src/features/chat/lib/sessionConfigRequests.ts new file mode 100644 index 0000000000..4b5a72dd3d --- /dev/null +++ b/ui/goose2/src/features/chat/lib/sessionConfigRequests.ts @@ -0,0 +1,168 @@ +import { acpPrepareSession, acpSetModel } from "@/shared/api/acp"; + +export interface SessionConfigRequest { + sessionId: string; + providerId: string; + workingDir: string; + modelId?: string | null; +} + +export interface SessionConfigResult { + applied: boolean; +} + +interface QueuedSessionConfigRequest extends SessionConfigRequest { + sequence: number; +} + +interface SessionConfigWaiter { + sequence: number; + request: QueuedSessionConfigRequest; + resolve: (result: SessionConfigResult) => void; + reject: (error: unknown) => void; +} + +interface SessionConfigQueue { + latest: QueuedSessionConfigRequest | null; + nextSequence: number; + running: boolean; + waiters: SessionConfigWaiter[]; +} + +const queues = new Map(); + +function getQueue(sessionId: string): SessionConfigQueue { + let queue = queues.get(sessionId); + if (!queue) { + queue = { + latest: null, + nextSequence: 0, + running: false, + waiters: [], + }; + queues.set(sessionId, queue); + } + return queue; +} + +function sameSessionConfig( + a: SessionConfigRequest, + b: SessionConfigRequest, +): boolean { + return ( + a.providerId === b.providerId && + a.workingDir === b.workingDir && + (a.modelId ?? null) === (b.modelId ?? null) + ); +} + +function settleFailedWaiters( + queue: SessionConfigQueue, + sequence: number, + error: unknown, +) { + const remaining: SessionConfigWaiter[] = []; + for (const waiter of queue.waiters) { + if (waiter.sequence > sequence) { + remaining.push(waiter); + continue; + } + + if (waiter.sequence === sequence) { + waiter.reject(error); + } else { + waiter.resolve({ applied: false }); + } + } + queue.waiters = remaining; +} + +function settleAppliedWaiters( + queue: SessionConfigQueue, + request: QueuedSessionConfigRequest, +) { + const remaining: SessionConfigWaiter[] = []; + for (const waiter of queue.waiters) { + if (waiter.sequence > request.sequence) { + remaining.push(waiter); + continue; + } + + waiter.resolve({ + applied: sameSessionConfig(waiter.request, request), + }); + } + queue.waiters = remaining; +} + +async function applyRequest(request: QueuedSessionConfigRequest) { + await acpPrepareSession( + request.sessionId, + request.providerId, + request.workingDir, + ); + if (request.modelId) { + await acpSetModel(request.sessionId, request.modelId); + } +} + +async function drainQueue(sessionId: string, queue: SessionConfigQueue) { + if (queue.running) { + return; + } + + queue.running = true; + try { + while (queue.latest) { + const request = queue.latest; + try { + await applyRequest(request); + } catch (error) { + if (queue.latest?.sequence !== request.sequence) { + continue; + } + + queue.latest = null; + settleFailedWaiters(queue, request.sequence, error); + break; + } + + if (queue.latest?.sequence !== request.sequence) { + continue; + } + + queue.latest = null; + settleAppliedWaiters(queue, request); + break; + } + } finally { + queue.running = false; + if (queue.latest) { + void drainQueue(sessionId, queue); + } else if (queue.waiters.length === 0) { + queues.delete(sessionId); + } + } +} + +export function applyLatestSessionConfig( + request: SessionConfigRequest, +): Promise { + const queue = getQueue(request.sessionId); + const sequence = queue.nextSequence + 1; + queue.nextSequence = sequence; + const queuedRequest = { ...request, sequence }; + queue.latest = queuedRequest; + + const result = new Promise((resolve, reject) => { + queue.waiters.push({ + sequence, + request: queuedRequest, + resolve, + reject, + }); + }); + + void drainQueue(request.sessionId, queue); + return result; +} diff --git a/ui/goose2/src/features/chat/lib/submitComposerMessage.ts b/ui/goose2/src/features/chat/lib/submitComposerMessage.ts index e8541b6dfb..ed83150ca3 100644 --- a/ui/goose2/src/features/chat/lib/submitComposerMessage.ts +++ b/ui/goose2/src/features/chat/lib/submitComposerMessage.ts @@ -1,7 +1,7 @@ import type { SkillCommandMatch } from "@/features/skills/lib/skillChatPrompt"; import { isPromiseLike } from "@/shared/lib/isPromiseLike"; import type { ChatAttachmentDraft } from "@/shared/types/messages"; -import type { ChatInputProps, ChatSkillDraft } from "../types"; +import type { ChatInputSendHandler, ChatSkillDraft } from "../types"; import { buildSkillSendPayload } from "./skillSendPayload"; interface SubmitComposerMessageOptions { @@ -9,7 +9,7 @@ interface SubmitComposerMessageOptions { attachments: ChatAttachmentDraft[]; skills: ChatSkillDraft[]; selectedPersonaId?: string | null; - onSend: ChatInputProps["onSend"]; + onSend: ChatInputSendHandler; resolveSkillSlashCommand: ( message: string, ) => SkillCommandMatch | null; diff --git a/ui/goose2/src/features/chat/types.ts b/ui/goose2/src/features/chat/types.ts index e084cb5a04..677d1cb3b0 100644 --- a/ui/goose2/src/features/chat/types.ts +++ b/ui/goose2/src/features/chat/types.ts @@ -35,26 +35,29 @@ export interface ChatSendOptions { chips?: MessageChip[]; } -export interface ChatInputProps { - onSend: ( - text: string, - personaId?: string, - attachments?: ChatAttachmentDraft[], - options?: ChatSendOptions, - ) => boolean | Promise; +export type ChatInputSendHandler = ( + text: string, + personaId?: string, + attachments?: ChatAttachmentDraft[], + options?: ChatSendOptions, +) => boolean | Promise; + +export interface ChatInputComposerActions { + onSend: ChatInputSendHandler; onStop?: () => void; isStreaming?: boolean; disabled?: boolean; queuedMessage?: { text: string } | null; onDismissQueue?: () => void; - initialValue?: string; - onDraftChange?: (text: string) => void; - selectedSkills?: ChatSkillDraft[]; - onSkillsChange?: (skills: ChatSkillDraft[]) => void; - className?: string; +} + +export interface ChatInputPersonaPicker { personas?: Persona[]; selectedPersonaId?: string | null; onPersonaChange?: (personaId: string | null) => void; +} + +export interface ChatInputAgentModelPicker { providers?: AcpProvider[]; providersLoading?: boolean; selectedProvider?: string; @@ -67,12 +70,18 @@ export interface ChatInputProps { modelStatusMessage?: string | null; onModelChange?: (modelId: string, model?: ModelOption) => void; onPickerOpen?: () => void; +} + +export interface ChatInputProjectPicker { selectedProjectId?: string | null; availableProjects?: ProjectOption[]; onProjectChange?: (projectId: string | null) => void; onCreateProject?: (options?: { onCreated?: (projectId: string) => void; }) => void; +} + +export interface ChatInputContextUsage { contextTokens?: number; contextLimit?: number; isContextUsageReady?: boolean; @@ -81,3 +90,16 @@ export interface ChatInputProps { isCompactingContext?: boolean; supportsCompactionControls?: boolean; } + +export interface ChatInputProps { + composerActions: ChatInputComposerActions; + initialValue?: string; + onDraftChange?: (text: string) => void; + selectedSkills?: ChatSkillDraft[]; + onSkillsChange?: (skills: ChatSkillDraft[]) => void; + className?: string; + personaPicker?: ChatInputPersonaPicker; + agentModelPicker?: ChatInputAgentModelPicker; + projectPicker?: ChatInputProjectPicker; + contextUsage?: ChatInputContextUsage; +} diff --git a/ui/goose2/src/features/chat/ui/AgentModelPicker.tsx b/ui/goose2/src/features/chat/ui/AgentModelPicker.tsx index 2accb0e0b7..e688e2fcd0 100644 --- a/ui/goose2/src/features/chat/ui/AgentModelPicker.tsx +++ b/ui/goose2/src/features/chat/ui/AgentModelPicker.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { IconCheck, IconChevronDown } from "@tabler/icons-react"; import { useTranslation } from "react-i18next"; import type { AcpProvider } from "@/shared/api/acp"; @@ -11,6 +11,10 @@ import { formatProviderLabel, getProviderIcon, } from "@/shared/ui/icons/ProviderIcons"; +import { + resolveDisplayModelLabel, + resolvePickerTriggerLabel, +} from "../lib/modelDisplayLabel"; import type { ModelOption } from "../types"; import { AllModelsList, RecommendedModelList } from "./AgentModelPickerLists"; import { PickerItem } from "./AgentModelPickerItem"; @@ -52,16 +56,26 @@ export function AgentModelPicker({ }: AgentModelPickerProps) { const { t } = useTranslation("chat"); const [open, setOpen] = useState(false); + const contentRef = useRef(null); const [modelView, setModelView] = useState("recommended"); const selectedAgentLabel = agents.find((agent) => agent.id === selectedAgentId)?.label ?? formatProviderLabel(selectedAgentId); - const hasSelectedModel = - showSelectedModelInTrigger && - (currentModelName !== null || currentModelId !== null); - const triggerModelLabel = hasSelectedModel - ? (currentModelName ?? currentModelId) - : null; + const displayModelLabel = resolveDisplayModelLabel({ + currentModelId, + currentModelName, + currentModelProviderId, + availableModels, + }); + const triggerLabel = showSelectedModelInTrigger + ? resolvePickerTriggerLabel({ + currentModelId, + currentModelName, + currentModelProviderId, + availableModels, + selectedAgentLabel, + }) + : selectedAgentLabel; const handleAgentSelect = (agentId: string) => { if (agentId !== selectedAgentId) { @@ -105,15 +119,22 @@ export function AgentModelPicker({ className="min-w-0 max-w-full" > - {triggerModelLabel ?? - selectedAgentLabel ?? - (loading ? t("toolbar.loading") : null)} + {triggerLabel ?? (loading ? t("toolbar.loading") : null)} { + e.preventDefault(); + contentRef.current + ?.querySelector( + '[data-col="agent"] button[data-selected]', + ) + ?.focus(); + }} onKeyDown={(e) => { if (e.key === "ArrowDown" || e.key === "ArrowUp") { e.preventDefault(); @@ -192,6 +213,7 @@ export function AgentModelPicker({ key={agent.id} onClick={() => handleAgentSelect(agent.id)} selected={isSelected} + data-selected={isSelected || undefined} > {agentIcon ? ( {agentIcon} @@ -221,11 +243,11 @@ export function AgentModelPicker({
{t("toolbar.model")}
- {currentModelName || currentModelId ? ( + {displayModelLabel ? (
- {currentModelName ?? currentModelId} + {displayModelLabel}
@@ -267,7 +289,7 @@ export function AgentModelPicker({
{modelStatusMessage ?? - currentModelName ?? + displayModelLabel ?? t("toolbar.noModelsAvailable")}
diff --git a/ui/goose2/src/features/chat/ui/AgentModelPickerItem.tsx b/ui/goose2/src/features/chat/ui/AgentModelPickerItem.tsx index c09fb37284..7e1e676693 100644 --- a/ui/goose2/src/features/chat/ui/AgentModelPickerItem.tsx +++ b/ui/goose2/src/features/chat/ui/AgentModelPickerItem.tsx @@ -1,4 +1,4 @@ -import type { ReactNode } from "react"; +import type { ButtonHTMLAttributes, ReactNode } from "react"; import { cn } from "@/shared/lib/cn"; export function PickerItem({ @@ -7,13 +7,11 @@ export function PickerItem({ selected = false, disabled = false, className, + ...rest }: { children: ReactNode; - onClick?: () => void; selected?: boolean; - disabled?: boolean; - className?: string; -}) { +} & Omit, "type">) { return ( diff --git a/ui/goose2/src/features/chat/ui/ChatInput.tsx b/ui/goose2/src/features/chat/ui/ChatInput.tsx index cfb72669f5..76674417fb 100644 --- a/ui/goose2/src/features/chat/ui/ChatInput.tsx +++ b/ui/goose2/src/features/chat/ui/ChatInput.tsx @@ -24,47 +24,63 @@ import { ChatInputAttachments } from "./ChatInputAttachments"; import { ChatInputSelectionChips } from "./ChatInputSelectionChips"; import { useChatInputSubmit } from "../hooks/useChatInputSubmit"; import { useVoiceDictation } from "../hooks/useVoiceDictation"; +import { resolveDisplayModelLabel } from "../lib/modelDisplayLabel"; import type { ChatInputProps, ChatSkillDraft } from "../types"; export function ChatInput({ - onSend, - onStop, - isStreaming = false, - disabled = false, - queuedMessage = null, - onDismissQueue, + composerActions, initialValue = "", onDraftChange, selectedSkills: selectedSkillsProp, onSkillsChange, className, - personas = [], - selectedPersonaId = null, - onPersonaChange, - providers = [], - providersLoading = false, - selectedProvider = "goose", - onProviderChange, - currentModelId = null, - currentModelProviderId = null, - currentModel, - availableModels = [], - modelsLoading = false, - modelStatusMessage = null, - onModelChange, - onPickerOpen, - selectedProjectId = null, - availableProjects = [], - onProjectChange, - onCreateProject, - contextTokens = 0, - contextLimit = 0, - isContextUsageReady, - onCompactContext, - canCompactContext = false, - isCompactingContext = false, - supportsCompactionControls, + personaPicker, + agentModelPicker, + projectPicker, + contextUsage, }: ChatInputProps) { + const { + onSend, + onStop, + isStreaming = false, + disabled = false, + queuedMessage = null, + onDismissQueue, + } = composerActions; + const { + personas = [], + selectedPersonaId = null, + onPersonaChange, + } = personaPicker ?? {}; + const { + providers = [], + providersLoading = false, + selectedProvider = "goose", + onProviderChange, + currentModelId = null, + currentModelProviderId = null, + currentModel, + availableModels = [], + modelsLoading = false, + modelStatusMessage = null, + onModelChange, + onPickerOpen, + } = agentModelPicker ?? {}; + const { + selectedProjectId = null, + availableProjects = [], + onProjectChange, + onCreateProject, + } = projectPicker ?? {}; + const { + contextTokens = 0, + contextLimit = 0, + isContextUsageReady, + onCompactContext, + canCompactContext = false, + isCompactingContext = false, + supportsCompactionControls, + } = contextUsage ?? {}; const { t } = useTranslation("chat"); const [text, setTextRaw] = useState(initialValue); const [internalSelectedSkills, setInternalSelectedSkills] = useState< @@ -339,17 +355,15 @@ export function ChatInput({ providerDisplayName, ); const resolvedCurrentModel = useMemo(() => { - if (currentModel) { - return currentModel; - } - if (!currentModelId) { - return undefined; - } - const selectedModel = availableModels.find( - (model) => model.id === currentModelId, + return ( + resolveDisplayModelLabel({ + currentModelId, + currentModelName: currentModel, + currentModelProviderId, + availableModels, + }) ?? undefined ); - return selectedModel?.displayName ?? selectedModel?.name ?? currentModelId; - }, [availableModels, currentModel, currentModelId]); + }, [availableModels, currentModel, currentModelId, currentModelProviderId]); const inputPlaceholder = getChatInputPlaceholder( t, agentDisplayName, @@ -457,43 +471,51 @@ export function ChatInput({ onProviderChange?.(id)} - currentModelId={currentModelId} - currentModelProviderId={currentModelProviderId} - currentModel={resolvedCurrentModel} - availableModels={availableModels} - modelsLoading={modelsLoading} - modelStatusMessage={modelStatusMessage} - onModelChange={onModelChange} - onPickerOpen={onPickerOpen} - selectedProjectId={selectedProjectId} - availableProjects={availableProjects} - onProjectChange={onProjectChange} - onCreateProject={onCreateProject} - contextTokens={contextTokens} - contextLimit={contextLimit} - isContextUsageReady={isContextUsageReady} - onCompactContext={onCompactContext} - canCompactContext={canCompactContext} - isCompactingContext={isCompactingContext} - supportsCompactionControls={supportsCompactionControls} - canSend={canSend} - isStreaming={isStreaming} - hasQueuedMessage={hasQueuedMessage} - onAttachFiles={handleAttachFiles} - onAttachFolders={handleAttachFolders} - disabled={disabled} - onSend={handleSend} - onStop={onStop} + personaPicker={{ selectedPersonaId }} + agentModelPicker={{ + providers, + providersLoading, + selectedProvider, + onProviderChange, + currentModelId, + currentModelProviderId, + currentModel: resolvedCurrentModel, + availableModels, + modelsLoading, + modelStatusMessage, + onModelChange, + onPickerOpen, + }} + projectPicker={{ + selectedProjectId, + availableProjects, + onProjectChange, + onCreateProject, + }} + contextUsage={{ + contextTokens, + contextLimit, + isContextUsageReady, + onCompactContext, + canCompactContext, + isCompactingContext, + supportsCompactionControls, + }} + composerActions={{ + canSend, + isStreaming, + hasQueuedMessage, + onAttachFiles: handleAttachFiles, + onAttachFolders: handleAttachFolders, + disabled, + onSend: handleSend, + onStop, + voiceEnabled: dictation.isEnabled, + voiceRecording: dictation.isRecording, + voiceTranscribing: dictation.isTranscribing, + onVoiceToggle: dictation.toggleRecording, + }} isCompact={isCompact} - voiceEnabled={dictation.isEnabled} - voiceRecording={dictation.isRecording} - voiceTranscribing={dictation.isTranscribing} - onVoiceToggle={dictation.toggleRecording} />
diff --git a/ui/goose2/src/features/chat/ui/ChatInputToolbar.tsx b/ui/goose2/src/features/chat/ui/ChatInputToolbar.tsx index e292a35e28..0a02c2c176 100644 --- a/ui/goose2/src/features/chat/ui/ChatInputToolbar.tsx +++ b/ui/goose2/src/features/chat/ui/ChatInputToolbar.tsx @@ -15,7 +15,6 @@ import type { AcpProvider } from "@/shared/api/acp"; import { cn } from "@/shared/lib/cn"; import { ChatInputSelector } from "./ChatInputSelector"; import { ContextRing } from "./ContextRing"; -import type { ProjectOption } from "../types"; import { Button } from "@/shared/ui/button"; import { DropdownMenu, @@ -27,49 +26,23 @@ import { Popover, PopoverContent, PopoverTrigger } from "@/shared/ui/popover"; import { Progress } from "@/shared/ui/progress"; import { Tooltip, TooltipTrigger, TooltipContent } from "@/shared/ui/tooltip"; import { AgentModelPicker } from "./AgentModelPicker"; -import type { ModelOption } from "../types"; import { formatProviderLabel } from "@/shared/ui/icons/ProviderIcons"; import { getCatalogEntryFromEntries } from "@/features/providers/providerCatalog"; import { useProviderCatalogStore } from "@/features/providers/stores/providerCatalogStore"; import { supportsContextCompactionControls } from "../lib/autoCompact"; import { requestOpenSettings } from "@/features/settings/lib/settingsEvents"; import { ProjectSelectorIcon } from "./ProjectSelectorIcon"; +import type { + ChatInputAgentModelPicker, + ChatInputContextUsage, + ChatInputPersonaPicker, + ChatInputProjectPicker, +} from "../types"; const NO_PROJECT_VALUE = "__no_project__"; const CREATE_PROJECT_VALUE = "__create_project__"; -interface ChatInputToolbarProps { - selectedPersonaId: string | null; - // Provider - providers: AcpProvider[]; - providersLoading?: boolean; - selectedProvider: string; - onProviderChange: (providerId: string) => void; - // Model - currentModelId?: string | null; - currentModelProviderId?: string | null; - currentModel?: string; - availableModels: ModelOption[]; - modelsLoading?: boolean; - modelStatusMessage?: string | null; - onModelChange?: (modelId: string, model?: ModelOption) => void; - onPickerOpen?: () => void; - // Project - selectedProjectId: string | null; - availableProjects: ProjectOption[]; - onProjectChange?: (projectId: string | null) => void; - onCreateProject?: (options?: { - onCreated?: (projectId: string) => void; - }) => void; - // Context - contextTokens: number; - contextLimit: number; - isContextUsageReady?: boolean; - supportsCompactionControls?: boolean; - // Actions - canCompactContext?: boolean; - isCompactingContext?: boolean; - onCompactContext?: () => Promise | undefined; +interface ChatInputToolbarComposerActions { canSend: boolean; isStreaming: boolean; hasQueuedMessage: boolean; @@ -78,58 +51,77 @@ interface ChatInputToolbarProps { onAttachFiles?: () => void; onAttachFolders?: () => void; disabled?: boolean; - // Voice voiceEnabled?: boolean; voiceRecording?: boolean; voiceTranscribing?: boolean; onVoiceToggle?: () => void; - // Layout +} + +interface ChatInputToolbarProps { + personaPicker: Pick; + agentModelPicker: ChatInputAgentModelPicker; + projectPicker: ChatInputProjectPicker; + contextUsage: ChatInputContextUsage; + composerActions: ChatInputToolbarComposerActions; isCompact: boolean; } export function ChatInputToolbar({ - selectedPersonaId, - providers, - providersLoading, - selectedProvider, - onProviderChange, - currentModelId, - currentModelProviderId, - currentModel, - availableModels, - modelsLoading = false, - modelStatusMessage = null, - onModelChange, - onPickerOpen, - selectedProjectId, - availableProjects, - onProjectChange, - onCreateProject, - contextTokens, - contextLimit, - isContextUsageReady, - supportsCompactionControls, - canCompactContext = false, - isCompactingContext = false, - onCompactContext, - canSend, - isStreaming, - hasQueuedMessage, - onSend, - onStop, - onAttachFiles, - onAttachFolders, - disabled = false, - voiceEnabled = false, - voiceRecording = false, - voiceTranscribing = false, - onVoiceToggle, + personaPicker, + agentModelPicker, + projectPicker, + contextUsage, + composerActions, isCompact, }: ChatInputToolbarProps) { const { t } = useTranslation("chat"); const { formatNumber } = useLocaleFormatting(); const catalogEntries = useProviderCatalogStore((state) => state.entries); const [isContextPopoverOpen, setIsContextPopoverOpen] = useState(false); + const { selectedPersonaId = null } = personaPicker; + const { + providers = [], + providersLoading, + selectedProvider = "goose", + onProviderChange, + currentModelId, + currentModelProviderId, + currentModel, + availableModels = [], + modelsLoading = false, + modelStatusMessage = null, + onModelChange, + onPickerOpen, + } = agentModelPicker; + const { + selectedProjectId = null, + availableProjects = [], + onProjectChange, + onCreateProject, + } = projectPicker; + const { + contextTokens = 0, + contextLimit = 0, + isContextUsageReady, + supportsCompactionControls, + canCompactContext = false, + isCompactingContext = false, + onCompactContext, + } = contextUsage; + const { + canSend, + isStreaming, + hasQueuedMessage, + onSend, + onStop, + onAttachFiles, + onAttachFolders, + disabled = false, + voiceEnabled = false, + voiceRecording = false, + voiceTranscribing = false, + onVoiceToggle, + } = composerActions; const compactionControlsSupported = supportsCompactionControls ?? supportsContextCompactionControls(selectedProvider); @@ -230,7 +222,7 @@ export function ChatInputToolbar({ onProviderChange?.(providerId)} currentModelId={currentModelId} currentModelProviderId={currentModelProviderId} currentModelName={currentModel ?? null} diff --git a/ui/goose2/src/features/chat/ui/ChatView.tsx b/ui/goose2/src/features/chat/ui/ChatView.tsx index 3c0e233dd0..74a3383703 100644 --- a/ui/goose2/src/features/chat/ui/ChatView.tsx +++ b/ui/goose2/src/features/chat/ui/ChatView.tsx @@ -125,55 +125,62 @@ export function ChatView({ - onCreateProject?.({ - onCreated: (projectId) => { - controller.handleProjectChange(projectId); - options?.onCreated?.(projectId); - }, - }) - } - contextTokens={controller.tokenState.accumulatedTotal} - contextLimit={controller.tokenState.contextLimit} - isContextUsageReady={controller.isContextUsageReady} - onCompactContext={controller.compactConversation} - canCompactContext={controller.canCompactContext} - isCompactingContext={controller.isCompactingContext} - supportsCompactionControls={controller.supportsCompactionControls} + personaPicker={{ + personas: controller.personas, + selectedPersonaId: controller.selectedPersonaId, + onPersonaChange: controller.handlePersonaChange, + }} + agentModelPicker={{ + providers: controller.pickerAgents, + providersLoading: controller.providersLoading, + selectedProvider: controller.selectedProvider, + onProviderChange: controller.handleProviderChange, + currentModelId: controller.currentModelId, + currentModelProviderId: controller.currentModelProviderId, + currentModel: controller.currentModelName ?? undefined, + availableModels: controller.availableModels, + modelsLoading: controller.modelsLoading, + modelStatusMessage: controller.modelStatusMessage, + onModelChange: controller.handleModelChange, + onPickerOpen: controller.handlePickerOpen, + }} + projectPicker={{ + selectedProjectId: controller.selectedProjectId, + availableProjects: controller.availableProjects, + onProjectChange: controller.handleProjectChange, + onCreateProject: (options) => + onCreateProject?.({ + onCreated: (projectId) => { + controller.handleProjectChange(projectId); + options?.onCreated?.(projectId); + }, + }), + }} + contextUsage={{ + contextTokens: controller.tokenState.accumulatedTotal, + contextLimit: controller.tokenState.contextLimit, + isContextUsageReady: controller.isContextUsageReady, + onCompactContext: controller.compactConversation, + canCompactContext: controller.canCompactContext, + isCompactingContext: controller.isCompactingContext, + supportsCompactionControls: controller.supportsCompactionControls, + }} /> diff --git a/ui/goose2/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx b/ui/goose2/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx index ebdcecff58..6a290931c1 100644 --- a/ui/goose2/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx +++ b/ui/goose2/src/features/chat/ui/__tests__/AgentModelPicker.test.tsx @@ -14,6 +14,7 @@ globalThis.ResizeObserver ??= const AGENTS = [ { id: "goose", label: "Goose" }, + { id: "claude-acp", label: "Claude Code" }, { id: "codex-acp", label: "Codex" }, ]; @@ -36,6 +37,108 @@ describe("AgentModelPicker", () => { ).toHaveTextContent("GPT-4o"); }); + it("uses the selected agent label while a raw model id is unresolved", () => { + render( + , + ); + + const trigger = screen.getByRole("button", { + name: /choose agent and model/i, + }); + expect(trigger).toHaveTextContent("Claude Code"); + expect(trigger).not.toHaveTextContent("opus"); + }); + + it("uses the inventory model label for a matching raw model id", () => { + render( + , + ); + + expect( + screen.getByRole("button", { name: /choose agent and model/i }), + ).toHaveTextContent("Claude Opus 4.6"); + }); + + it("uses a stored human model name before inventory resolves", () => { + render( + , + ); + + expect( + screen.getByRole("button", { name: /choose agent and model/i }), + ).toHaveTextContent("Claude Opus 4.6"); + }); + + it("allows id-as-display-name labels after inventory resolves", () => { + render( + , + ); + + expect( + screen.getByRole("button", { name: /choose agent and model/i }), + ).toHaveTextContent("gpt-5.4"); + }); + + it("does not show a raw model id in the loading row", async () => { + const user = userEvent.setup(); + + render( + , + ); + + await user.click( + screen.getByRole("button", { name: /choose agent and model/i }), + ); + + expect(screen.getByText("Loading models...")).toBeInTheDocument(); + expect(screen.queryByText("opus")).not.toBeInTheDocument(); + }); + it("calls onModelChange when a model is selected", async () => { const user = userEvent.setup(); const onModelChange = vi.fn(); diff --git a/ui/goose2/src/features/chat/ui/__tests__/ChatInput.asyncSend.test.tsx b/ui/goose2/src/features/chat/ui/__tests__/ChatInput.asyncSend.test.tsx index a5f09301b6..79462c9549 100644 --- a/ui/goose2/src/features/chat/ui/__tests__/ChatInput.asyncSend.test.tsx +++ b/ui/goose2/src/features/chat/ui/__tests__/ChatInput.asyncSend.test.tsx @@ -1,7 +1,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { ChatInput } from "../ChatInput"; +import { ChatInput } from "./chatInputTestUtils"; const mockVoiceDictation = { isEnabled: true, diff --git a/ui/goose2/src/features/chat/ui/__tests__/ChatInput.attachments.test.tsx b/ui/goose2/src/features/chat/ui/__tests__/ChatInput.attachments.test.tsx index fb0138b6c9..1f6a7e7e63 100644 --- a/ui/goose2/src/features/chat/ui/__tests__/ChatInput.attachments.test.tsx +++ b/ui/goose2/src/features/chat/ui/__tests__/ChatInput.attachments.test.tsx @@ -7,7 +7,7 @@ import { waitFor, } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { ChatInput } from "../ChatInput"; +import { ChatInput } from "./chatInputTestUtils"; vi.mock("@/features/providers/hooks/useAgentProviderStatus", () => ({ useAgentProviderStatus: () => ({ diff --git a/ui/goose2/src/features/chat/ui/__tests__/ChatInput.skills.test.tsx b/ui/goose2/src/features/chat/ui/__tests__/ChatInput.skills.test.tsx index 38e6584c20..5bd396bf46 100644 --- a/ui/goose2/src/features/chat/ui/__tests__/ChatInput.skills.test.tsx +++ b/ui/goose2/src/features/chat/ui/__tests__/ChatInput.skills.test.tsx @@ -1,7 +1,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { act, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { ChatInput } from "../ChatInput"; +import { ChatInput } from "./chatInputTestUtils"; const mockVoiceDictation = { isEnabled: true, diff --git a/ui/goose2/src/features/chat/ui/__tests__/ChatInput.test.tsx b/ui/goose2/src/features/chat/ui/__tests__/ChatInput.test.tsx index 9933a480f6..39be5e01b2 100644 --- a/ui/goose2/src/features/chat/ui/__tests__/ChatInput.test.tsx +++ b/ui/goose2/src/features/chat/ui/__tests__/ChatInput.test.tsx @@ -2,7 +2,7 @@ import { beforeEach, describe, it, expect, vi } from "vitest"; import { fireEvent, render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { useState } from "react"; -import { ChatInput } from "../ChatInput"; +import { ChatInput } from "./chatInputTestUtils"; import { ChatInputToolbar } from "../ChatInputToolbar"; import { OPEN_SETTINGS_EVENT } from "@/features/settings/lib/settingsEvents"; import type { Persona } from "@/shared/types/agents"; @@ -167,6 +167,26 @@ describe("ChatInput", () => { ).toHaveTextContent("Goose"); }); + it("shows provider label while the current model id is unresolved", () => { + render( + , + ); + + const trigger = screen.getByRole("button", { + name: /choose agent and model/i, + }); + expect(trigger).toHaveTextContent("Claude Code"); + expect(trigger).not.toHaveTextContent("opus"); + }); + it("shows default provider label", () => { render( { it("keeps the mic toggle enabled while recording even if voice input becomes unavailable", () => { render( , ); diff --git a/ui/goose2/src/features/chat/ui/__tests__/chatInputTestUtils.tsx b/ui/goose2/src/features/chat/ui/__tests__/chatInputTestUtils.tsx new file mode 100644 index 0000000000..7ed27895f1 --- /dev/null +++ b/ui/goose2/src/features/chat/ui/__tests__/chatInputTestUtils.tsx @@ -0,0 +1,110 @@ +import { ChatInput as BaseChatInput } from "../ChatInput"; +import type { + ChatInputAgentModelPicker, + ChatInputComposerActions, + ChatInputContextUsage, + ChatInputPersonaPicker, + ChatInputProjectPicker, + ChatInputProps, + ChatInputSendHandler, +} from "../../types"; + +type ChatInputHarnessProps = Omit< + ChatInputProps, + | "composerActions" + | "personaPicker" + | "agentModelPicker" + | "projectPicker" + | "contextUsage" +> & + Partial & + ChatInputPersonaPicker & + ChatInputAgentModelPicker & + ChatInputProjectPicker & + ChatInputContextUsage & { + onSend: ChatInputSendHandler; + }; + +export function ChatInput({ + onSend, + onStop, + isStreaming, + disabled, + queuedMessage, + onDismissQueue, + personas, + selectedPersonaId, + onPersonaChange, + providers, + providersLoading, + selectedProvider, + onProviderChange, + currentModelId, + currentModelProviderId, + currentModel, + availableModels, + modelsLoading, + modelStatusMessage, + onModelChange, + onPickerOpen, + selectedProjectId, + availableProjects, + onProjectChange, + onCreateProject, + contextTokens, + contextLimit, + isContextUsageReady, + onCompactContext, + canCompactContext, + isCompactingContext, + supportsCompactionControls, + ...props +}: ChatInputHarnessProps) { + return ( + + ); +} diff --git a/ui/goose2/src/features/home/ui/HomeScreen.tsx b/ui/goose2/src/features/home/ui/HomeScreen.tsx index 1b8e9c6106..0ee94f6ffd 100644 --- a/ui/goose2/src/features/home/ui/HomeScreen.tsx +++ b/ui/goose2/src/features/home/ui/HomeScreen.tsx @@ -66,47 +66,56 @@ function HomeComposer({ return ( - onCreateProject?.({ - onCreated: (projectId) => { - controller.handleProjectChange(projectId); - options?.onCreated?.(projectId); - }, - }) - } - contextTokens={controller.tokenState.accumulatedTotal} - contextLimit={controller.tokenState.contextLimit} - isContextUsageReady={controller.isContextUsageReady} + personaPicker={{ + personas: controller.personas, + selectedPersonaId: controller.selectedPersonaId, + onPersonaChange: controller.handlePersonaChange, + }} + agentModelPicker={{ + providers: controller.pickerAgents, + providersLoading: controller.providersLoading, + selectedProvider: controller.selectedProvider, + onProviderChange: controller.handleProviderChange, + currentModelId: controller.currentModelId, + currentModelProviderId: controller.currentModelProviderId, + currentModel: controller.currentModelName ?? undefined, + availableModels: controller.availableModels, + modelsLoading: controller.modelsLoading, + modelStatusMessage: controller.modelStatusMessage, + onModelChange: controller.handleModelChange, + onPickerOpen: controller.handlePickerOpen, + }} + projectPicker={{ + selectedProjectId: controller.selectedProjectId, + availableProjects: controller.availableProjects, + onProjectChange: controller.handleProjectChange, + onCreateProject: (options) => + onCreateProject?.({ + onCreated: (projectId) => { + controller.handleProjectChange(projectId); + options?.onCreated?.(projectId); + }, + }), + }} + contextUsage={{ + contextTokens: controller.tokenState.accumulatedTotal, + contextLimit: controller.tokenState.contextLimit, + isContextUsageReady: controller.isContextUsageReady, + }} /> ); } diff --git a/ui/goose2/src/shared/i18n/locales/en/chat.json b/ui/goose2/src/shared/i18n/locales/en/chat.json index 976f26d884..c06d25d059 100644 --- a/ui/goose2/src/shared/i18n/locales/en/chat.json +++ b/ui/goose2/src/shared/i18n/locales/en/chat.json @@ -13,6 +13,9 @@ "openPanel": "Open context panel", "ringAria": "Context: {{percent}}% used" }, + "errors": { + "sessionPreparationSuperseded": "Session configuration changed while preparing. Try sending again." + }, "contextPanel": { "actions": { "chooseCreateAction": "Choose create action", diff --git a/ui/goose2/src/shared/i18n/locales/es/chat.json b/ui/goose2/src/shared/i18n/locales/es/chat.json index 16e6b70ede..5433024058 100644 --- a/ui/goose2/src/shared/i18n/locales/es/chat.json +++ b/ui/goose2/src/shared/i18n/locales/es/chat.json @@ -13,6 +13,9 @@ "openPanel": "Abrir panel de contexto", "ringAria": "Contexto: {{percent}}% usado" }, + "errors": { + "sessionPreparationSuperseded": "La configuración de la sesión cambió durante la preparación. Intenta enviar de nuevo." + }, "contextPanel": { "actions": { "chooseCreateAction": "Elegir acción de creación",