From 33d71d687f602b9bcec99ea7ceb6ef190e2c03f1 Mon Sep 17 00:00:00 2001 From: Austin Pickett Date: Sun, 19 Jul 2026 19:31:34 -0400 Subject: [PATCH] fix(desktop): preserve new-chat selector choices (#67729) Salvaged and rebased from #66354 by @UnathiCodex onto current main. Fixes a fresh-chat race in Hermes Desktop where a model, reasoning-effort, or Fast selection made before the first Send could be replaced by an in-flight profile refresh, or read only after the profile handshake yielded. Send is now the linearization point: the visible selector state is snapshotted before awaiting profile readiness, and intent-generation guards make older config/model responses stand down after a picker/toggle action. Adds the contract-v4 session-create wire contract for explicit Fast=false. Conflict resolution vs the original branch (use-model-controls.ts / .test.tsx): combined main's catalog-aware keepManualPick() sticky-pick logic with the PR's profileRefreshEpoch + composerSelectionGeneration staleness guards so both a removed-from-catalog reseed and the in-flight-picker race are handled. Verified on current main: apps/desktop tsc --noEmit clean; 80 affected UI/store tests pass (use-model-controls, use-hermes-config, use-session-actions, model-edit-submenu, model-presets, updates). Co-authored-by: UnathiCodex --- apps/desktop/src/app/contrib/wiring.tsx | 8 +- .../session/hooks/use-hermes-config.test.ts | 90 ++++++++++++++- .../app/session/hooks/use-hermes-config.ts | 107 +++++++++++------- .../session/hooks/use-model-controls.test.tsx | 63 +++++++++++ .../app/session/hooks/use-model-controls.ts | 26 ++++- .../hooks/use-session-actions.test.tsx | 81 ++++++++++++- .../hooks/use-session-actions/index.ts | 23 ++-- .../src/app/shell/model-edit-submenu.test.tsx | 24 +++- .../src/app/shell/model-edit-submenu.tsx | 9 +- apps/desktop/src/store/model-presets.test.ts | 11 +- apps/desktop/src/store/model-presets.ts | 16 +-- apps/desktop/src/store/session.ts | 13 +++ apps/desktop/src/store/updates.test.ts | 6 +- apps/desktop/src/store/updates.ts | 3 +- tests/test_tui_gateway_server.py | 95 +++++++++++++++- tui_gateway/server.py | 38 +++++-- 16 files changed, 527 insertions(+), 86 deletions(-) diff --git a/apps/desktop/src/app/contrib/wiring.tsx b/apps/desktop/src/app/contrib/wiring.tsx index a548e191bbb..fe77f99e15c 100644 --- a/apps/desktop/src/app/contrib/wiring.tsx +++ b/apps/desktop/src/app/contrib/wiring.tsx @@ -433,11 +433,13 @@ export function ContribWiring({ children }: { children: ReactNode }) { } lastGatewayProfileRef.current = activeGatewayProfile - // Force: the new profile has its own default, so reseed even if the - // composer already shows the previous profile's model. + // Force: the new profile has its own defaults, so reseed the selector even + // if the composer already shows values from the previous profile. Both + // refreshes carry an intent token so a picker click made in flight wins. void refreshCurrentModel(true) + void refreshHermesConfig(true) void refreshActiveProfile() - }, [activeGatewayProfile, refreshCurrentModel]) + }, [activeGatewayProfile, refreshCurrentModel, refreshHermesConfig]) // New session anchored to a workspace (sidebar "+" on a project/worktree). // Seeds cwd + branch from the clicked workspace; an explicit worktree path diff --git a/apps/desktop/src/app/session/hooks/use-hermes-config.test.ts b/apps/desktop/src/app/session/hooks/use-hermes-config.test.ts index f4c6878b7f6..576fcad5ccd 100644 --- a/apps/desktop/src/app/session/hooks/use-hermes-config.test.ts +++ b/apps/desktop/src/app/session/hooks/use-hermes-config.test.ts @@ -4,7 +4,16 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { getHermesConfig } from '@/hermes' import { persistString } from '@/lib/storage' -import { $currentCwd, setCurrentCwd } from '@/store/session' +import { + $currentCwd, + $currentFastMode, + $currentReasoningEffort, + markComposerSelectionManual, + setCurrentCwd, + setCurrentFastMode, + setCurrentModelSource, + setCurrentReasoningEffort +} from '@/store/session' import { useHermesConfig } from './use-hermes-config' @@ -15,6 +24,16 @@ vi.mock('@/hermes', () => ({ const WORKSPACE_CWD_KEY = 'hermes.desktop.workspace-cwd' +function deferred() { + let resolve!: (value: T | PromiseLike) => void + + const promise = new Promise(done => { + resolve = done + }) + + return { promise, resolve } +} + const mockConfig = (config: Record) => vi.mocked(getHermesConfig).mockResolvedValue(config as Awaited>) @@ -22,6 +41,9 @@ describe('useHermesConfig refreshHermesConfig', () => { beforeEach(() => { // Reset atoms and localStorage between tests setCurrentCwd('') + setCurrentFastMode(false) + setCurrentModelSource('') + setCurrentReasoningEffort('') persistString(WORKSPACE_CWD_KEY, null) }) @@ -141,4 +163,70 @@ describe('useHermesConfig refreshHermesConfig', () => { expect(refreshProjectBranch).toHaveBeenCalledWith('/workspace/attached-project') }) + + it('does not let a stale forced config refresh overwrite newer draft selector intent', async () => { + const profileConfig = deferred>>() + vi.mocked(getHermesConfig).mockReturnValueOnce(profileConfig.promise) + + const { result } = renderHook(() => + useHermesConfig({ + activeSessionIdRef: { current: null }, + refreshProjectBranch: vi.fn().mockResolvedValue(undefined) + }) + ) + + let pendingRefresh!: Promise + act(() => { + pendingRefresh = result.current.refreshHermesConfig(true) + }) + expect(getHermesConfig).toHaveBeenCalled() + + // The user turns Fast off and chooses a different effort while the profile + // defaults are still loading. That newer picker intent owns the composer. + markComposerSelectionManual() + setCurrentReasoningEffort('high') + setCurrentFastMode(false) + profileConfig.resolve({ + agent: { reasoning_effort: 'low', service_tier: 'priority' } + } as Awaited>) + + await act(async () => { + await pendingRefresh + }) + + expect($currentReasoningEffort.get()).toBe('high') + expect($currentFastMode.get()).toBe(false) + }) + + it('does not let an older profile config overwrite a newer profile', async () => { + const profileB = deferred>>() + const profileC = deferred>>() + vi.mocked(getHermesConfig).mockReturnValueOnce(profileB.promise).mockReturnValueOnce(profileC.promise) + + const { result } = renderHook(() => + useHermesConfig({ + activeSessionIdRef: { current: null }, + refreshProjectBranch: vi.fn().mockResolvedValue(undefined) + }) + ) + + let refreshB!: Promise + let refreshC!: Promise + act(() => { + refreshB = result.current.refreshHermesConfig(true) + refreshC = result.current.refreshHermesConfig(true) + }) + + profileC.resolve({ agent: { reasoning_effort: 'low', service_tier: 'normal' } }) + await act(async () => { + await refreshC + }) + profileB.resolve({ agent: { reasoning_effort: 'high', service_tier: 'priority' } }) + await act(async () => { + await refreshB + }) + + expect($currentReasoningEffort.get()).toBe('low') + expect($currentFastMode.get()).toBe(false) + }) }) diff --git a/apps/desktop/src/app/session/hooks/use-hermes-config.ts b/apps/desktop/src/app/session/hooks/use-hermes-config.ts index 8c3cbac65a3..52681e20a85 100644 --- a/apps/desktop/src/app/session/hooks/use-hermes-config.ts +++ b/apps/desktop/src/app/session/hooks/use-hermes-config.ts @@ -1,10 +1,12 @@ -import { type MutableRefObject, useCallback, useState } from 'react' +import { type MutableRefObject, useCallback, useRef, useState } from 'react' import { getHermesConfig, getHermesConfigDefaults } from '@/hermes' import { BUILTIN_PERSONALITIES, normalizePersonalityValue, personalityNamesFromConfig } from '@/lib/chat-runtime' import { normalize } from '@/lib/text' import { $currentCwd, + getComposerSelectionGeneration, + getCurrentModelSource, setAvailablePersonalities, setCurrentCwd, setCurrentFastMode, @@ -47,51 +49,74 @@ interface HermesConfigOptions { export function useHermesConfig({ activeSessionIdRef, refreshProjectBranch }: HermesConfigOptions) { const [voiceMaxRecordingSeconds, setVoiceMaxRecordingSeconds] = useState(DEFAULT_VOICE_SECONDS) const [sttEnabled, setSttEnabled] = useState(true) + const profileRefreshEpochRef = useRef(0) - const refreshHermesConfig = useCallback(async () => { - try { - const [config, defaults] = await Promise.all([getHermesConfig(), getHermesConfigDefaults().catch(() => ({}))]) - - const personality = normalizePersonalityValue( - typeof config.display?.personality === 'string' ? config.display.personality : '' - ) - - setIntroPersonality(personality) - // Active sessions keep their per-session value; standalone falls back to config. - setCurrentPersonality(prev => (activeSessionIdRef.current ? prev || personality : personality)) - setAvailablePersonalities([ - ...new Set([ - 'none', - ...BUILTIN_PERSONALITIES, - ...personalityNamesFromConfig(defaults), - ...personalityNamesFromConfig(config) - ]) - ]) - - const cwd = (config.terminal?.cwd ?? '').trim() - - if (cwd && cwd !== '.') { - // Configured terminal.cwd beats a stale remembered workspace cwd - // (#38855) — but never yank the workspace out from under an active - // session; those keep their own cwd until the user detaches. - setCurrentCwd(prev => (activeSessionIdRef.current ? prev : cwd)) - void refreshProjectBranch($currentCwd.get() || cwd) + const refreshHermesConfig = useCallback( + async (force = false) => { + if (force) { + profileRefreshEpochRef.current += 1 } - const reasoning = normalizeConfigEffort(config.agent?.reasoning_effort) - const tier = (config.agent?.service_tier ?? '').trim() + const profileRefreshEpoch = profileRefreshEpochRef.current + const selectionGeneration = getComposerSelectionGeneration() - setCurrentReasoningEffort(prev => (activeSessionIdRef.current ? prev : reasoning)) - setCurrentServiceTier(prev => (activeSessionIdRef.current ? prev : tier)) - setCurrentFastMode(prev => (activeSessionIdRef.current ? prev : FAST_TIERS.has(tier.toLowerCase()))) + try { + const [config, defaults] = await Promise.all([getHermesConfig(), getHermesConfigDefaults().catch(() => ({}))]) - setVoiceMaxRecordingSeconds(recordingLimit(config.voice?.max_recording_seconds)) - setSttEnabled(config.stt?.enabled !== false) - applyAutoSpeakFromConfig(config) - } catch { - // Config is nice-to-have; chat still works without it. - } - }, [activeSessionIdRef, refreshProjectBranch]) + if (profileRefreshEpochRef.current !== profileRefreshEpoch) { + return + } + + const personality = normalizePersonalityValue( + typeof config.display?.personality === 'string' ? config.display.personality : '' + ) + + setIntroPersonality(personality) + // Active sessions keep their per-session value; standalone falls back to config. + setCurrentPersonality(prev => (activeSessionIdRef.current ? prev || personality : personality)) + setAvailablePersonalities([ + ...new Set([ + 'none', + ...BUILTIN_PERSONALITIES, + ...personalityNamesFromConfig(defaults), + ...personalityNamesFromConfig(config) + ]) + ]) + + const cwd = (config.terminal?.cwd ?? '').trim() + + if (cwd && cwd !== '.') { + // Configured terminal.cwd beats a stale remembered workspace cwd + // (#38855) — but never yank the workspace out from under an active + // session; those keep their own cwd until the user detaches. + setCurrentCwd(prev => (activeSessionIdRef.current ? prev : cwd)) + void refreshProjectBranch($currentCwd.get() || cwd) + } + + const reasoning = normalizeConfigEffort(config.agent?.reasoning_effort) + const tier = (config.agent?.service_tier ?? '').trim() + + const shouldSeedComposer = + !activeSessionIdRef.current && + getComposerSelectionGeneration() === selectionGeneration && + (force || getCurrentModelSource() !== 'manual') + + if (shouldSeedComposer) { + setCurrentReasoningEffort(reasoning) + setCurrentFastMode(FAST_TIERS.has(tier.toLowerCase())) + } + + setCurrentServiceTier(prev => (activeSessionIdRef.current ? prev : tier)) + + setVoiceMaxRecordingSeconds(recordingLimit(config.voice?.max_recording_seconds)) + setSttEnabled(config.stt?.enabled !== false) + applyAutoSpeakFromConfig(config) + } catch { + // Config is nice-to-have; chat still works without it. + } + }, + [activeSessionIdRef, refreshProjectBranch] + ) return { refreshHermesConfig, sttEnabled, voiceMaxRecordingSeconds } } diff --git a/apps/desktop/src/app/session/hooks/use-model-controls.test.tsx b/apps/desktop/src/app/session/hooks/use-model-controls.test.tsx index 6e1f459d1c1..f5ab213d045 100644 --- a/apps/desktop/src/app/session/hooks/use-model-controls.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-model-controls.test.tsx @@ -18,6 +18,16 @@ import { useModelControls } from './use-model-controls' const setGlobalModel = vi.fn() const notifyError = vi.fn() +function deferred() { + let resolve!: (value: T | PromiseLike) => void + + const promise = new Promise(done => { + resolve = done + }) + + return { promise, resolve } +} + vi.mock('@/hermes', () => ({ getGlobalModelInfo: vi.fn(), setGlobalModel: (...args: Parameters) => setGlobalModel(...args) @@ -248,6 +258,59 @@ describe('useModelControls', () => { expect(getCurrentModelSource()).toBe('manual') }) + it('does not let a stale forced profile refresh overwrite a newer picker choice', async () => { + const profileDefault = deferred>>() + vi.mocked(getGlobalModelInfo).mockReturnValueOnce(profileDefault.promise) + + const { result } = renderHook(() => + useModelControls({ + queryClient: new QueryClient(), + requestGateway: vi.fn() + }) + ) + + const pendingRefresh = result.current.refreshCurrentModel(true) + expect(getGlobalModelInfo).toHaveBeenCalled() + + await expect( + result.current.selectModel({ + model: 'claude-sonnet-4.6', + provider: 'anthropic' + }) + ).resolves.toBe(true) + + profileDefault.resolve({ model: 'gpt-5.5', provider: 'openai-codex' }) + await pendingRefresh + + expect($currentModel.get()).toBe('claude-sonnet-4.6') + expect($currentProvider.get()).toBe('anthropic') + expect(getCurrentModelSource()).toBe('manual') + }) + + it('does not let an older profile refresh overwrite a newer profile', async () => { + const profileB = deferred>>() + const profileC = deferred>>() + vi.mocked(getGlobalModelInfo).mockReturnValueOnce(profileB.promise).mockReturnValueOnce(profileC.promise) + + const { result } = renderHook(() => + useModelControls({ + queryClient: new QueryClient(), + requestGateway: vi.fn() + }) + ) + + const refreshB = result.current.refreshCurrentModel(true) + const refreshC = result.current.refreshCurrentModel(true) + + profileC.resolve({ model: 'profile-c-model', provider: 'profile-c-provider' }) + await refreshC + profileB.resolve({ model: 'profile-b-model', provider: 'profile-b-provider' }) + await refreshB + + expect($currentModel.get()).toBe('profile-c-model') + expect($currentProvider.get()).toBe('profile-c-provider') + }) + it('refreshes legacy/default-derived composer state from the profile default', async () => { setCurrentModel('openai/gpt-5.5') setCurrentProvider('nous') diff --git a/apps/desktop/src/app/session/hooks/use-model-controls.ts b/apps/desktop/src/app/session/hooks/use-model-controls.ts index edc2747e599..c7f4d245cb7 100644 --- a/apps/desktop/src/app/session/hooks/use-model-controls.ts +++ b/apps/desktop/src/app/session/hooks/use-model-controls.ts @@ -1,5 +1,5 @@ import { type QueryClient } from '@tanstack/react-query' -import { useCallback } from 'react' +import { useCallback, useRef } from 'react' import { getGlobalModelInfo } from '@/hermes' import { useI18n } from '@/i18n' @@ -9,7 +9,9 @@ import { $activeSessionId, $currentModel, $currentProvider, + getComposerSelectionGeneration, getCurrentModelSource, + markComposerSelectionManual, setCurrentModel, setCurrentModelSource, setCurrentProvider @@ -29,6 +31,7 @@ interface ModelControlsOptions { export function useModelControls({ queryClient, requestGateway }: ModelControlsOptions) { const { t } = useI18n() const copy = t.desktop + const profileRefreshEpochRef = useRef(0) // All callbacks here read reactive session state from the store (.get()) // rather than capturing it as a prop. The actions bag in wiring.tsx mutates @@ -55,6 +58,14 @@ export function useModelControls({ queryClient, requestGateway }: ModelControlsO // draft / session events. A live session owns the footer, so skip entirely. const refreshCurrentModel = useCallback( async (force = false) => { + // A forced profile swap opens a new intent epoch; an older in-flight + // response for a previous profile must stand down when it resolves. + if (force) { + profileRefreshEpochRef.current += 1 + } + + const profileRefreshEpoch = profileRefreshEpochRef.current + try { if ($activeSessionId.get()) { return @@ -79,9 +90,18 @@ export function useModelControls({ queryClient, requestGateway }: ModelControlsO return } + // Snapshot the selection generation before awaiting so a picker click + // that lands while getGlobalModelInfo is in flight wins over this older + // default — value comparisons alone miss re-selecting the same row. + const selectionGeneration = getComposerSelectionGeneration() const result = await getGlobalModelInfo() - if ($activeSessionId.get() || keepManualPick()) { + if ( + profileRefreshEpochRef.current !== profileRefreshEpoch || + $activeSessionId.get() || + getComposerSelectionGeneration() !== selectionGeneration || + keepManualPick() + ) { return } @@ -122,7 +142,7 @@ export function useModelControls({ queryClient, requestGateway }: ModelControlsO setCurrentModel(selection.model) setCurrentProvider(selection.provider) - setCurrentModelSource('manual') + markComposerSelectionManual() updateModelOptionsCache(selection.provider, selection.model, !liveSessionId) // No live session yet: the pick is pure UI state. session.create reads diff --git a/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx b/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx index f9288eb8686..d13717ad48e 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx @@ -5,12 +5,16 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { getSessionMessages, type SessionInfo } from '@/hermes' import { createClientSessionState } from '@/lib/chat-runtime' -import { $activeGatewayProfile, $newChatProfile } from '@/store/profile' +import { $activeGatewayProfile, $newChatProfile, ensureGatewayProfile } from '@/store/profile' import { $projectScope, $projectTree, ALL_PROJECTS } from '@/store/projects' import { $activeSessionId, $activeSessionStoredIdRotation, $currentCwd, + $currentFastMode, + $currentModel, + $currentProvider, + $currentReasoningEffort, $messages, $newChatWorkspaceTarget, $resumeFailedSessionId, @@ -18,6 +22,10 @@ import { setActiveSessionId, setActiveSessionStoredIdRotation, setCurrentCwd, + setCurrentFastMode, + setCurrentModel, + setCurrentProvider, + setCurrentReasoningEffort, setMessages, setNewChatWorkspaceTarget, setResumeFailedSessionId, @@ -39,7 +47,23 @@ vi.mock('@/hermes', async importOriginal => ({ setSessionArchived: vi.fn() })) +vi.mock('@/store/profile', async importOriginal => ({ + ...(await importOriginal>()), + ensureGatewayProfile: vi.fn().mockResolvedValue(undefined) +})) + const RUNTIME_SESSION_ID = 'rt-new-001' + +function deferred() { + let resolve!: (value: T | PromiseLike) => void + + const promise = new Promise(done => { + resolve = done + }) + + return { promise, resolve } +} + type HarnessHandle = Pick< ReturnType, 'createBackendSessionForSend' | 'startFreshSessionDraft' @@ -304,6 +328,10 @@ describe('createBackendSessionForSend profile routing', () => { $projectScope.set(ALL_PROJECTS) $projectTree.set([]) $currentCwd.set('') + $currentFastMode.set(false) + $currentModel.set('') + $currentProvider.set('') + $currentReasoningEffort.set('') setNewChatWorkspaceTarget(undefined) vi.restoreAllMocks() }) @@ -353,6 +381,57 @@ describe('createBackendSessionForSend profile routing', () => { expect(params).toMatchObject({ cwd: '/remote/worktree' }) }) + it('freezes the visible selector state before profile readiness and sends fast: false explicitly', async () => { + const profileReady = deferred() + vi.mocked(ensureGatewayProfile).mockReturnValueOnce(profileReady.promise) + + setCurrentModel('anthropic/claude-sonnet-4.6') + setCurrentProvider('anthropic') + setCurrentReasoningEffort('high') + setCurrentFastMode(false) + + let createParams: Record | undefined + + const requestGateway = vi.fn(async (method: string, params?: Record) => { + if (method === 'session.create') { + createParams = params + + return { session_id: RUNTIME_SESSION_ID, stored_session_id: null } as never + } + + return {} as never + }) + + let handle: HarnessHandle | null = null + render( (handle = next)} requestGateway={requestGateway} />) + await waitFor(() => expect(handle).not.toBeNull()) + + let createPromise!: Promise + act(() => { + createPromise = handle!.createBackendSessionForSend() + }) + await waitFor(() => expect(ensureGatewayProfile).toHaveBeenCalled()) + + // A background refresh or a second click can mutate the sticky atoms while + // the profile is waking. This send must still use what was visible at Enter. + setCurrentModel('openai/gpt-5.5') + setCurrentProvider('openai-codex') + setCurrentReasoningEffort('low') + setCurrentFastMode(true) + profileReady.resolve() + + await act(async () => { + await createPromise + }) + + expect(createParams).toMatchObject({ + fast: false, + model: 'anthropic/claude-sonnet-4.6', + provider: 'anthropic', + reasoning_effort: 'high' + }) + }) + it('falls back to the entered project cwd when the current cwd is blank', async () => { const params = await createWith(() => { $projectTree.set([ diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts index 9f48aeed5a5..6934d00208c 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts @@ -147,21 +147,30 @@ function reconcileAuthoritativeMessages( // profile to None). The sticky UI model/effort/fast ride as per-session overrides, // never the profile default (that lives in Settings → Model). async function desktopSessionCreateParams(cwd: string): Promise> { + // Treat Send as the linearization point for the visible selector state. The + // profile handshake below can yield long enough for background config/model + // refreshes to finish; reading atoms afterward would silently create the + // session with a different selection than the one the user submitted. + const selection = { + effort: $currentReasoningEffort.get().trim(), + fast: $currentFastMode.get(), + model: $currentModel.get().trim(), + provider: $currentProvider.get().trim() + } + const profile = $newChatProfile.get() ?? normalizeProfileKey($activeGatewayProfile.get()) await ensureGatewayProfile(profile) - const model = $currentModel.get().trim() - const provider = $currentProvider.get().trim() - const effort = $currentReasoningEffort.get().trim() - return { cols: 96, source: 'desktop', ...(cwd && { cwd }), ...(profile ? { profile } : {}), - ...(model ? { model, ...(provider ? { provider } : {}) } : {}), - ...(effort ? { reasoning_effort: effort } : {}), - ...($currentFastMode.get() ? { fast: true } : {}) + ...(selection.model + ? { model: selection.model, ...(selection.provider ? { provider: selection.provider } : {}) } + : {}), + ...(selection.effort ? { reasoning_effort: selection.effort } : {}), + fast: selection.fast } } diff --git a/apps/desktop/src/app/shell/model-edit-submenu.test.tsx b/apps/desktop/src/app/shell/model-edit-submenu.test.tsx index 4358b75f8c7..c46e38aa240 100644 --- a/apps/desktop/src/app/shell/model-edit-submenu.test.tsx +++ b/apps/desktop/src/app/shell/model-edit-submenu.test.tsx @@ -8,7 +8,15 @@ import { DropdownMenuSubTrigger } from '@/components/ui/dropdown-menu' import { $modelPresets, getModelPreset } from '@/store/model-presets' -import { $activeSessionId } from '@/store/session' +import { + $activeSessionId, + $currentFastMode, + $currentReasoningEffort, + getCurrentModelSource, + setCurrentFastMode, + setCurrentModelSource, + setCurrentReasoningEffort +} from '@/store/session' import { type FastControl, ModelEditSubmenu } from './model-edit-submenu' @@ -22,6 +30,9 @@ beforeAll(() => { beforeEach(() => { $modelPresets.set({}) $activeSessionId.set(null) + setCurrentFastMode(false) + setCurrentModelSource('') + setCurrentReasoningEffort('') }) afterEach(() => { @@ -56,13 +67,16 @@ function renderSubmenu(opts: { fastControl: FastControl; reasoning: boolean; req // preset-only — the gateway's config.set falls back to global config when no // session matches, so it must not be called. (Caught in the second review.) describe('ModelEditSubmenu no-session guard', () => { - it('param fast: records the preset but skips the gateway without a session', () => { + it('param fast: records explicit off in the draft but skips the gateway without a session', () => { const requestGateway = vi.fn().mockResolvedValue({}) - renderSubmenu({ fastControl: { kind: 'param', on: false }, reasoning: false, requestGateway }) + setCurrentFastMode(true) + renderSubmenu({ fastControl: { kind: 'param', on: true }, reasoning: false, requestGateway }) fireEvent.click(screen.getByRole('switch')) - expect(getModelPreset('p1', 'm1').fast).toBe(true) + expect(getModelPreset('p1', 'm1').fast).toBe(false) + expect($currentFastMode.get()).toBe(false) + expect(getCurrentModelSource()).toBe('manual') expect(requestGateway).not.toHaveBeenCalled() }) @@ -74,6 +88,8 @@ describe('ModelEditSubmenu no-session guard', () => { fireEvent.click(screen.getByRole('switch')) expect(getModelPreset('p1', 'm1').effort).toBe('none') + expect($currentReasoningEffort.get()).toBe('none') + expect(getCurrentModelSource()).toBe('manual') expect(requestGateway).not.toHaveBeenCalled() }) diff --git a/apps/desktop/src/app/shell/model-edit-submenu.tsx b/apps/desktop/src/app/shell/model-edit-submenu.tsx index dda409699f5..527c84cec45 100644 --- a/apps/desktop/src/app/shell/model-edit-submenu.tsx +++ b/apps/desktop/src/app/shell/model-edit-submenu.tsx @@ -15,7 +15,12 @@ import { useI18n } from '@/i18n' import { normalize } from '@/lib/text' import { setModelPreset } from '@/store/model-presets' import { notifyError } from '@/store/notifications' -import { $activeSessionId, setCurrentFastMode, setCurrentReasoningEffort } from '@/store/session' +import { + $activeSessionId, + markComposerSelectionManual, + setCurrentFastMode, + setCurrentReasoningEffort +} from '@/store/session' // Hermes' real reasoning levels (see VALID_REASONING_EFFORTS); `none` is owned // by the Thinking toggle, not the radio. @@ -120,6 +125,7 @@ export function ModelEditSubmenu({ return } + markComposerSelectionManual() setCurrentReasoningEffort(next) // Preset-only without a session: `isActive` holds for the global/default @@ -161,6 +167,7 @@ export function ModelEditSubmenu({ return } + markComposerSelectionManual() setCurrentFastMode(enabled) // Preset-only without a session (see patchReasoning). diff --git a/apps/desktop/src/store/model-presets.test.ts b/apps/desktop/src/store/model-presets.test.ts index efe49ffa6e5..ef37cecc07d 100644 --- a/apps/desktop/src/store/model-presets.test.ts +++ b/apps/desktop/src/store/model-presets.test.ts @@ -1,9 +1,14 @@ import { beforeEach, describe, expect, it } from 'vitest' import { $modelPresets, applyModelPreset, getModelPreset, modelPresetKey, setModelPreset } from './model-presets' +import { $currentFastMode, $currentReasoningEffort, setCurrentFastMode, setCurrentReasoningEffort } from './session' describe('model presets', () => { - beforeEach(() => $modelPresets.set({})) + beforeEach(() => { + $modelPresets.set({}) + setCurrentFastMode(false) + setCurrentReasoningEffort('') + }) it('round-trips a preset and merges patches without dropping prior fields', () => { setModelPreset('anthropic', 'claude-opus-4-8', { effort: 'high' }) @@ -35,7 +40,7 @@ describe('model presets', () => { expect(calls).toEqual([{ method: 'config.set', params: { key: 'reasoning', session_id: 's1', value: 'high' } }]) }) - it('no-ops without a session so selecting a model cannot mutate global config', async () => { + it('applies a fresh-draft preset locally without mutating gateway config', async () => { const calls: { method: string; params?: Record }[] = [] const request = async (method: string, params?: Record) => { @@ -46,6 +51,8 @@ describe('model presets', () => { await applyModelPreset({ effort: 'high', fast: true }, { failMessage: 'x', request, sessionId: null }) + expect($currentReasoningEffort.get()).toBe('high') + expect($currentFastMode.get()).toBe(true) expect(calls).toEqual([]) }) }) diff --git a/apps/desktop/src/store/model-presets.ts b/apps/desktop/src/store/model-presets.ts index 9a66a8b0d2c..8771b38e082 100644 --- a/apps/desktop/src/store/model-presets.ts +++ b/apps/desktop/src/store/model-presets.ts @@ -51,19 +51,15 @@ export function setModelPreset(provider: string, model: string, patch: ModelPres persistString(STORAGE_KEY, JSON.stringify(next)) } -/** Push a model's preset onto the active session (optimistic + gateway). +/** Apply a model's preset to the composer, then push it to a live session. * `undefined` skips that dimension; values are capability-gated upstream. - * No-ops without a session — the gateway's `config.set` reasoning/fast fall - * back to persistent (global/profile) config when none matches, so selecting - * a model must not reach it (else it rewrites `agent.*`, defaults included). */ + * Without a session the local draft still needs the preset, but must not call + * `config.set`: that falls back to persistent profile config when no session + * matches and would rewrite the user's defaults. */ export async function applyModelPreset( { effort, fast }: ModelPreset, ctx: { failMessage: string; request: RequestGateway; sessionId: null | string } ): Promise { - if (!ctx.sessionId) { - return - } - if (effort !== undefined) { setCurrentReasoningEffort(effort) } @@ -72,6 +68,10 @@ export async function applyModelPreset( setCurrentFastMode(fast) } + if (!ctx.sessionId) { + return + } + try { if (effort !== undefined) { await ctx.request('config.set', { key: 'reasoning', session_id: ctx.sessionId, value: effort }) diff --git a/apps/desktop/src/store/session.ts b/apps/desktop/src/store/session.ts index 07ce07c02a4..dbfd7dbc799 100644 --- a/apps/desktop/src/store/session.ts +++ b/apps/desktop/src/store/session.ts @@ -381,6 +381,19 @@ export const setCurrentModelSource = (source: ComposerModelSource) => { $currentModelSource.set(source) } +// Monotonic intent token for async default refreshes. A profile/config request +// may start before the user opens the picker and finish after their click; the +// token lets that older response stand down even when the selected value is +// unchanged (value comparisons alone cannot detect re-selecting the same row). +let composerSelectionGeneration = 0 + +export const getComposerSelectionGeneration = (): number => composerSelectionGeneration + +export const markComposerSelectionManual = (): void => { + composerSelectionGeneration += 1 + setCurrentModelSource('manual') +} + export const setCurrentReasoningEffort = (next: Updater) => { updateAtom($currentReasoningEffort, next) persistString(COMPOSER_EFFORT_KEY, $currentReasoningEffort.get() || null) diff --git a/apps/desktop/src/store/updates.test.ts b/apps/desktop/src/store/updates.test.ts index 7439a8a8345..1a0dc7f0792 100644 --- a/apps/desktop/src/store/updates.test.ts +++ b/apps/desktop/src/store/updates.test.ts @@ -120,7 +120,7 @@ describe('reportBackendContract', () => { }) it('dismisses the toast when the backend meets the contract', () => { - reportBackendContract(3) + reportBackendContract(4) expect(dismissSpy).toHaveBeenCalledWith('backend-contract-skew') expect(notifySpy).not.toHaveBeenCalled() }) @@ -160,8 +160,8 @@ describe('reportBackendContract', () => { lastToast().onDismiss() notifySpy.mockClear() - reportBackendContract(3) // backend updated → satisfied, snooze cleared - reportBackendContract(2) // a later regression must warn immediately + reportBackendContract(4) // backend updated → satisfied, snooze cleared + reportBackendContract(3) // a later regression must warn immediately expect(notifySpy).toHaveBeenCalledTimes(1) }) }) diff --git a/apps/desktop/src/store/updates.ts b/apps/desktop/src/store/updates.ts index a7b0bbc8b94..6c7e1483cf9 100644 --- a/apps/desktop/src/store/updates.ts +++ b/apps/desktop/src/store/updates.ts @@ -92,7 +92,8 @@ function isUpdateToastSnoozed(): boolean { // value (or none — a pre-GUI checkout) means GUI<->backend skew. // v2: requires the file.attach RPC (remote-gateway non-image file upload). // v3: requires approvals.mode config RPCs and session.info reconciliation. -const REQUIRED_BACKEND_CONTRACT = 3 +// v4: requires explicit Fast-off session creation and session-scoped Fast edits. +const REQUIRED_BACKEND_CONTRACT = 4 const SKEW_TOAST_ID = 'backend-contract-skew' // The contract check runs on every session.resume (applyRuntimeInfo), so // without a snooze the warning re-popped on every thread the user opened, even diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 71d709f5dc6..786587b0790 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -1757,6 +1757,18 @@ def test_stored_session_runtime_overrides_skips_bare_billing_provider(): assert ov["model_override"]["provider"] == "custom:myendpoint" +def test_stored_session_runtime_overrides_restores_explicit_normal_tier(): + overrides = server._stored_session_runtime_overrides( + { + "model": "gpt-5.4", + "model_config": {"service_tier": "normal"}, + } + ) + + assert "service_tier_override" in overrides + assert overrides["service_tier_override"] == "" + + def test_persist_live_session_runtime_preserves_resume_metadata(monkeypatch): updates = {} @@ -1796,6 +1808,37 @@ def test_persist_live_session_runtime_preserves_resume_metadata(monkeypatch): ) +def test_persist_live_session_runtime_preserves_explicit_normal_tier(): + updates = {} + + class FakeDB: + def get_session(self, _session_id): + return {"model_config": '{"service_tier":"priority"}'} + + def update_session_meta(self, _session_id, model_config_json, model=None): + updates["config"] = json.loads(model_config_json) + + agent = types.SimpleNamespace( + model="gpt-5.4", + provider="openai-codex", + base_url=None, + api_mode=None, + reasoning_config=None, + service_tier="", + _session_db=FakeDB(), + ) + + server._persist_live_session_runtime( + { + "agent": agent, + "session_key": "stored-session", + "create_service_tier_override": "", + } + ) + + assert updates["config"]["service_tier"] == "normal" + + def test_status_callback_emits_kind_and_text(): with patch("tui_gateway.server._emit") as emit: cb = server._agent_cbs("sid")["status_callback"] @@ -10267,8 +10310,16 @@ def test_session_create_records_ui_model_as_session_override(monkeypatch): assert resp["result"]["info"]["model"] == "claude-sonnet-4.6" assert resp["result"]["info"]["provider"] == "anthropic" + # Explicit false is not the same as omission: it must suppress a Fast + # profile default for this session's first request. + normal = server._methods["session.create"]( + "r2", {"cols": 80, "fast": False} + ) + normal_sess = server._sessions[normal["result"]["session_id"]] + assert normal_sess["create_service_tier_override"] == "" + # No knobs → no overrides; the session builds from the profile default. - plain = server._methods["session.create"]("r2", {"cols": 80}) + plain = server._methods["session.create"]("r3", {"cols": 80}) plain_sess = server._sessions[plain["result"]["session_id"]] assert plain_sess["model_override"] is None assert plain_sess["create_reasoning_override"] is None @@ -10277,7 +10328,10 @@ def test_session_create_records_ui_model_as_session_override(monkeypatch): server._sessions.clear() -def test_start_agent_build_passes_session_model_override(monkeypatch): +@pytest.mark.parametrize("service_tier_override", ["priority", ""]) +def test_start_agent_build_passes_session_model_override( + monkeypatch, service_tier_override +): """A model staged on the session (e.g. by session.create from the desktop composer) must reach _make_agent so the first build runs on it directly — no global config, no build-then-switch. @@ -10317,7 +10371,7 @@ def test_start_agent_build_passes_session_model_override(monkeypatch): "profile_home": None, "model_override": override, "create_reasoning_override": reasoning, - "create_service_tier_override": "priority", + "create_service_tier_override": service_tier_override, } server._sessions[sid] = session try: @@ -10325,7 +10379,7 @@ def test_start_agent_build_passes_session_model_override(monkeypatch): assert session["agent_ready"].wait(timeout=3), "agent build did not finish" assert captured.get("model_override") == override assert captured.get("reasoning_config_override") == reasoning - assert captured.get("service_tier_override") == "priority" + assert captured.get("service_tier_override") == service_tier_override assert session["agent"].model == "claude-sonnet-4.6" finally: server._sessions.clear() @@ -10334,6 +10388,39 @@ def test_start_agent_build_passes_session_model_override(monkeypatch): # ── billing/subscription state + error serialization ───────────────── +def test_reset_session_agent_preserves_explicit_normal_fast(monkeypatch): + captured = {} + new_agent = types.SimpleNamespace(model="openai/gpt-5.4", service_tier="") + session = _session( + agent=types.SimpleNamespace( + model="openai/gpt-5.4", + reasoning_config=None, + service_tier="", + ), + model_override={"model": "openai/gpt-5.4"}, + create_service_tier_override="", + ) + + def make_agent(*_args, **kwargs): + captured.update(kwargs) + return new_agent + + monkeypatch.setattr(server, "_set_session_context", lambda _key: []) + monkeypatch.setattr(server, "_clear_session_context", lambda _tokens: None) + monkeypatch.setattr(server, "_make_agent", make_agent) + monkeypatch.setattr(server, "_config_model_target", lambda: ("", "")) + monkeypatch.setattr(server, "_load_show_reasoning", lambda: True) + monkeypatch.setattr(server, "_load_tool_progress_mode", lambda: "all") + monkeypatch.setattr(server, "_session_info", lambda *_args: {}) + monkeypatch.setattr(server, "_emit", lambda *_args: None) + monkeypatch.setattr(server, "_restart_slash_worker", lambda *_args: None) + + server._reset_session_agent("sid", session) + + assert captured["service_tier_override"] == "" + assert session["agent"] is new_agent + + @pytest.mark.parametrize( "card,expected", [ diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 5f04efaabd9..801f855910c 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -1984,8 +1984,13 @@ def _ensure_session_db_row(session: dict) -> None: ) if (reasoning := session.get("create_reasoning_override")) is not None: model_config["reasoning_config"] = reasoning - if tier := session.get("create_service_tier_override"): - model_config["service_tier"] = tier + create_service_tier_override = session.get("create_service_tier_override") + if create_service_tier_override is not None: + # Empty string is the in-memory sentinel for an explicit normal tier: + # it bypasses _make_agent's profile fallback without sending a bogus + # service_tier value to the provider. Persist a durable marker so resume + # can distinguish that choice from an omitted/inherited tier. + model_config["service_tier"] = create_service_tier_override or "normal" # Branch lineage: stamp the same ``_branched_from`` marker the TUI /branch # uses so list_sessions_rich keeps the branch listed and the desktop sidebar # can nest it under its parent. @@ -2604,7 +2609,11 @@ def _stored_session_runtime_overrides(row: dict | None) -> dict: overrides["provider_override"] = provider if isinstance(reasoning_config, dict): overrides["reasoning_config_override"] = reasoning_config - if service_tier: + if service_tier.lower() == "normal": + # None means "inherit the profile" at _make_agent. Empty string is a + # real override that means "do not request a priority service tier". + overrides["service_tier_override"] = "" + elif service_tier: overrides["service_tier_override"] = service_tier return overrides @@ -2692,6 +2701,12 @@ def _persist_live_session_runtime(session: dict | None) -> None: if isinstance(parsed, dict): existing_config = parsed model_config = _runtime_model_config(agent, existing_config) + create_service_tier_override = session.get("create_service_tier_override") + if create_service_tier_override is not None: + # _runtime_model_config sees agent.service_tier=None for explicit + # normal and would otherwise erase the distinction on every live + # metadata persist. + model_config["service_tier"] = create_service_tier_override or "normal" model = str(getattr(agent, "model", "") or "").strip() if hasattr(db, "update_session_meta"): db.update_session_meta(session_key, json.dumps(model_config), model or None) @@ -3686,7 +3701,8 @@ def _current_profile_name() -> str: # cryptically downstream. Bump whenever the desktop's backend contract changes. # v2: adds the file.attach RPC (remote-gateway non-image file upload). # v3: adds approvals.mode config RPCs and session.info reconciliation. -DESKTOP_BACKEND_CONTRACT = 3 +# v4: session.create fast=false is an explicit per-session normal-tier override. +DESKTOP_BACKEND_CONTRACT = 4 def _session_usage_snapshot(session: dict | None) -> dict: @@ -4758,6 +4774,9 @@ def _reset_session_agent(sid: str, session: dict) -> dict: old_reasoning = session.get("create_reasoning_override") if isinstance(old_reasoning, dict): reset_kw["reasoning_config_override"] = old_reasoning + create_service_tier_override = session.get("create_service_tier_override") + if create_service_tier_override is not None: + reset_kw["service_tier_override"] = create_service_tier_override new_agent = _make_agent( sid, session["session_key"], @@ -5759,9 +5778,14 @@ def _(rid, params: dict) -> dict: create_reasoning_override = parse_reasoning_effort(effort) except Exception: create_reasoning_override = None - # Only pin "fast" when explicitly requested; leaving it None lets the build - # fall back to the profile default service tier rather than forcing normal. - create_service_tier_override = "priority" if params.get("fast") else None + # Presence is part of the contract: omitted means inherit the profile, + # true pins priority, and false pins normal. Empty string is the internal + # explicit-normal sentinel because _make_agent uses None for inheritance. + create_service_tier_override = None + if "fast" in params: + create_service_tier_override = ( + "priority" if is_truthy_value(params.get("fast")) else "" + ) ready = threading.Event() now = time.time()