From 90ed53e3f9c0600ca8c08097070666ab2ab44010 Mon Sep 17 00:00:00 2001 From: Bob Lee Date: Sun, 2 Aug 2026 03:02:09 -0700 Subject: [PATCH] fix(web): stop a stale model-migration notice from reverting the picked model Picking a model in the composer appeared to do nothing on the first switch after client startup; a second click on the same model made it stick. The first explicit model update for a session is what pulls that session off disk. If its persisted model is no longer configured, restore repoints it to "auto" and broadcasts SessionModelAutoMigrated. That notice therefore lands *after* the composer already wrote the newly picked model optimistically, and the handler applied it unconditionally, reverting the selection. The second click found the session already in memory, so no restore, no notice, no revert. The backend itself switched correctly; only the store was rolled back. That is not cosmetic: syncSessionModelSelection pushes the store value before every send, so an unnoticed revert would overwrite the correct backend model with "auto" on the next turn. Apply the notice as a compare-and-swap instead: migrate only while the session still holds the model the backend migrated away from (or holds no selection). The CLI already guards this way in modes/chat/selection.rs; the desktop UI was the only consumer applying these events blindly. Background restores that genuinely need the migration still match and still apply. Also stop the selector from reporting a failed switch as a successful one: roll back the optimistic write and surface an error toast when the update rejects. --- .../flow_chat/components/ModelSelector.tsx | 22 +++++- .../flow-chat-manager/EventHandlerModule.ts | 17 ++++- .../src/flow_chat/store/FlowChatStore.test.ts | 67 +++++++++++++++++++ .../src/flow_chat/store/FlowChatStore.ts | 32 +++++++++ src/web-ui/src/locales/en-US/flow-chat.json | 1 + src/web-ui/src/locales/zh-CN/flow-chat.json | 1 + src/web-ui/src/locales/zh-TW/flow-chat.json | 1 + 7 files changed, 137 insertions(+), 4 deletions(-) diff --git a/src/web-ui/src/flow_chat/components/ModelSelector.tsx b/src/web-ui/src/flow_chat/components/ModelSelector.tsx index 5d7763e0d8..9b3fc21b38 100644 --- a/src/web-ui/src/flow_chat/components/ModelSelector.tsx +++ b/src/web-ui/src/flow_chat/components/ModelSelector.tsx @@ -20,6 +20,7 @@ import { getEffectiveReasoningMode, isReasoningVisiblyEnabled } from '@/infrastr import { globalEventBus } from '@/infrastructure/event-bus'; import type { AIModelConfig, AgentModelDefaultsConfig, DefaultModelsConfig } from '@/infrastructure/config/types'; import { Switch, Tooltip } from '@/component-library'; +import { notificationService } from '@/shared/notification-system'; import { FlowChatStore } from '../store/FlowChatStore'; import { getModelMaxTokens } from '../services/flow-chat-manager/SessionModule'; import { acpClientIdFromAgentType } from '../utils/acpSession'; @@ -570,6 +571,15 @@ export const ModelSelector: React.FC = ({ setLoading(true); setDropdownOpen(false); + // The optimistic session write below must be undone when the backend + // rejects the switch; otherwise the selector keeps showing a model the + // session never adopted, and the next send pushes it to the backend. + const store = FlowChatStore.getInstance(); + const previousSessionModelName = sessionId + ? store.getState().sessions.get(sessionId)?.config.modelName + : undefined; + let sessionModelWrittenOptimistically = false; + try { if (externalSelection) { await externalSelection.onSelect(modelId); @@ -586,7 +596,7 @@ export const ModelSelector: React.FC = ({ }); setAcpOptions(options); syncAcpContextUsageToStore(sessionId, options); - FlowChatStore.getInstance().updateSessionModelName(sessionId, modelId); + store.updateSessionModelName(sessionId, modelId); log.info('ACP session model updated', { sessionId, acpClientId, modelId }); return; } @@ -594,10 +604,10 @@ export const ModelSelector: React.FC = ({ const updateTargetSessionModel = async () => { if (!sessionId) return; - const store = FlowChatStore.getInstance(); // Update the frontend session model immediately so the UI reflects the // switch without waiting for the backend IPC round-trip. store.updateSessionModelName(sessionId, modelId); + sessionModelWrittenOptimistically = true; const maxContextTokens = await getModelMaxTokens(modelId, currentMode); store.updateSessionMaxContextTokens(sessionId, maxContextTokens); const session = store.getState().sessions.get(sessionId); @@ -628,6 +638,13 @@ export const ModelSelector: React.FC = ({ globalEventBus.emit('mode:config:updated'); } catch (error) { log.error('Failed to switch model', error); + // Only a previously pinned selection can be restored: the store has no + // way to express "never pinned", and forcing 'auto' there would claim a + // binding the session does not have either. + if (sessionId && sessionModelWrittenOptimistically && previousSessionModelName) { + store.updateSessionModelName(sessionId, previousSessionModelName); + } + notificationService.error(t('modelSelector.switchFailed')); } finally { setLoading(false); } @@ -642,6 +659,7 @@ export const ModelSelector: React.FC = ({ isAcpSession, loading, sessionId, + t, targetIsSubagent, ]); diff --git a/src/web-ui/src/flow_chat/services/flow-chat-manager/EventHandlerModule.ts b/src/web-ui/src/flow_chat/services/flow-chat-manager/EventHandlerModule.ts index a03342752d..4663d8139b 100644 --- a/src/web-ui/src/flow_chat/services/flow-chat-manager/EventHandlerModule.ts +++ b/src/web-ui/src/flow_chat/services/flow-chat-manager/EventHandlerModule.ts @@ -1181,11 +1181,24 @@ function handleSessionTitleGenerated(event: any): void { } function handleSessionModelAutoMigrated(event: SessionModelAutoMigratedEvent): void { - const { sessionId, newModelId } = event; + const { sessionId, previousModelId, newModelId, reason } = event; if (!sessionId || !newModelId) return; const store = FlowChatStore.getInstance(); - store.updateSessionModelName(sessionId, newModelId); + const applied = store.applySessionModelAutoMigration( + sessionId, + previousModelId ?? '', + newModelId, + ); + if (!applied) { + log.debug('Ignoring stale session model migration', { + sessionId, + previousModelId, + newModelId, + reason, + currentModelId: store.getState().sessions.get(sessionId)?.config.modelName, + }); + } } /** diff --git a/src/web-ui/src/flow_chat/store/FlowChatStore.test.ts b/src/web-ui/src/flow_chat/store/FlowChatStore.test.ts index c9bbf7f0e2..8e1242a404 100644 --- a/src/web-ui/src/flow_chat/store/FlowChatStore.test.ts +++ b/src/web-ui/src/flow_chat/store/FlowChatStore.test.ts @@ -1154,6 +1154,73 @@ describe('FlowChatStore session model selection', () => { expect(flowChatStore.getState().sessions.get(session.sessionId)?.config.modelName).toBe('auto'); }); + + it('applies an auto-migration notice that matches the stored model', () => { + const session = createSession({ + config: { agentType: 'agentic', modelName: 'removed-model' }, + }); + flowChatStore.setState(() => ({ + sessions: new Map([[session.sessionId, session]]), + activeSessionId: session.sessionId, + })); + + const applied = flowChatStore.applySessionModelAutoMigration( + session.sessionId, + 'removed-model', + 'auto', + ); + + expect(applied).toBe(true); + expect(flowChatStore.getState().sessions.get(session.sessionId)?.config.modelName).toBe('auto'); + }); + + it('applies an auto-migration notice when the session has no stored model yet', () => { + const session = createSession({ config: { agentType: 'agentic' } }); + flowChatStore.setState(() => ({ + sessions: new Map([[session.sessionId, session]]), + activeSessionId: session.sessionId, + })); + + const applied = flowChatStore.applySessionModelAutoMigration( + session.sessionId, + 'removed-model', + 'auto', + ); + + expect(applied).toBe(true); + expect(flowChatStore.getState().sessions.get(session.sessionId)?.config.modelName).toBe('auto'); + }); + + it('ignores a stale auto-migration notice that would revert a newer selection', () => { + // Restore-time migration races the explicit update that triggered the + // restore: the composer already stored the picked model when the notice + // for the old one lands. + const session = createSession({ + config: { agentType: 'agentic', modelName: 'removed-model' }, + }); + flowChatStore.setState(() => ({ + sessions: new Map([[session.sessionId, session]]), + activeSessionId: session.sessionId, + })); + + flowChatStore.updateSessionModelName(session.sessionId, 'deepseek-v4-flash'); + const applied = flowChatStore.applySessionModelAutoMigration( + session.sessionId, + 'removed-model', + 'auto', + ); + + expect(applied).toBe(false); + expect(flowChatStore.getState().sessions.get(session.sessionId)?.config.modelName).toBe( + 'deepseek-v4-flash', + ); + }); + + it('ignores an auto-migration notice for an unknown session', () => { + expect( + flowChatStore.applySessionModelAutoMigration('missing-session', 'removed-model', 'auto'), + ).toBe(false); + }); }); describe('FlowChatStore historical session hydration state', () => { diff --git a/src/web-ui/src/flow_chat/store/FlowChatStore.ts b/src/web-ui/src/flow_chat/store/FlowChatStore.ts index b86d752bf3..a511d40492 100644 --- a/src/web-ui/src/flow_chat/store/FlowChatStore.ts +++ b/src/web-ui/src/flow_chat/store/FlowChatStore.ts @@ -2188,6 +2188,38 @@ export class FlowChatStore { }); } + /** + * Apply a backend `SessionModelAutoMigrated` notice as a compare-and-swap. + * + * The backend emits this while restoring a session whose persisted model is + * gone. That restore is frequently triggered by the very model update the + * user just made, so the notice can land *after* the composer already stored + * the newly picked model. Applying it blindly reverts the user's choice, and + * the reverted value is what the next send pushes back to the backend. + * + * Only migrate while the session still holds the model the backend migrated + * away from (or holds no selection yet). Mirrors the CLI guard in + * `src/apps/cli/src/modes/chat/selection.rs`. + * + * Returns whether the migration was applied. + */ + public applySessionModelAutoMigration( + sessionId: string, + previousModelId: string, + newModelId: string, + ): boolean { + const session = this.state.sessions.get(sessionId); + if (!session) return false; + + const currentModelName = session.config.modelName?.trim(); + if (currentModelName && currentModelName !== previousModelId.trim()) { + return false; + } + + this.updateSessionModelName(sessionId, newModelId); + return true; + } + /** Update the target-owned model choice before an observer job is submitted. */ public updateSessionDispatchModel(sessionId: string, modelName: string): void { this.setState(prev => { diff --git a/src/web-ui/src/locales/en-US/flow-chat.json b/src/web-ui/src/locales/en-US/flow-chat.json index 24f8e60569..37c4d19eb4 100644 --- a/src/web-ui/src/locales/en-US/flow-chat.json +++ b/src/web-ui/src/locales/en-US/flow-chat.json @@ -2357,6 +2357,7 @@ "fastModel": "Fast Model", "fastMode": "Fast mode", "fastModeDescription": "1.5x speed with higher credit usage", + "switchFailed": "Failed to switch model", "modelNotConfigured": "Not Configured", "contextUsage": { "agentPrompt": "Last request prompt: {{usage}}", diff --git a/src/web-ui/src/locales/zh-CN/flow-chat.json b/src/web-ui/src/locales/zh-CN/flow-chat.json index 61e9246ede..35d39997e9 100644 --- a/src/web-ui/src/locales/zh-CN/flow-chat.json +++ b/src/web-ui/src/locales/zh-CN/flow-chat.json @@ -2357,6 +2357,7 @@ "fastModel": "Fast 模型", "fastMode": "Fast 模式", "fastModeDescription": "1.5 倍速度，消耗更多额度", + "switchFailed": "切换模型失败", "modelNotConfigured": "未配置", "contextUsage": { "agentPrompt": "上次请求输入上下文: {{usage}}", diff --git a/src/web-ui/src/locales/zh-TW/flow-chat.json b/src/web-ui/src/locales/zh-TW/flow-chat.json index 54c29061a6..7c775a51da 100644 --- a/src/web-ui/src/locales/zh-TW/flow-chat.json +++ b/src/web-ui/src/locales/zh-TW/flow-chat.json @@ -2357,6 +2357,7 @@ "fastModel": "Fast 模型", "fastMode": "Fast 模式", "fastModeDescription": "1.5 倍速度，消耗更多額度", + "switchFailed": "切換模型失敗", "modelNotConfigured": "未設定", "contextUsage": { "agentPrompt": "上次請求輸入上下文: {{usage}}",