Skip to content

Commit 339414b

Browse files
committed
fix(chat): resolve model selections by configuration ID
Match configured model IDs exclusively in the selector and pre-send synchronization to prevent same-name models from switching accounts. Preserve primary/fast selectors and block sending when a pinned model ID is unavailable or ambiguous. Add regression coverage for model identity collisions, unavailable selections, context windows, and remote routing parameters. Fixes #3007
1 parent a17c02b commit 339414b

6 files changed

Lines changed: 154 additions & 14 deletions

File tree

‎src/web-ui/src/flow_chat/components/ModelSelector.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -841,6 +841,8 @@ export const ModelSelector: React.FC<ModelSelectorProps> = ({
841841
status = 'unconfigured';
842842
} else if (availableModels.length === 0) {
843843
status = 'no-enabled-chat-model';
844+
} else if (!nativeModelResolution.model) {
845+
status = 'target-model-unavailable';
844846
} else if (catalogLoadState === 'error') {
845847
status = 'catalog-unavailable';
846848
} else if (nativeModelResolution.recovered) {
@@ -851,13 +853,14 @@ export const ModelSelector: React.FC<ModelSelectorProps> = ({
851853

852854
return {
853855
status,
854-
canSend: configLoadState === 'ready' && availableModels.length > 0,
856+
canSend: configLoadState === 'ready' && nativeModelResolution.model !== null,
855857
};
856858
}, [
857859
allModels.length,
858860
availableModels.length,
859861
catalogLoadState,
860862
configLoadState,
863+
nativeModelResolution.model,
861864
nativeModelResolution.recovered,
862865
]);
863866

‎src/web-ui/src/flow_chat/components/ModelSelectorProviderLevels.test.tsx‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,29 @@ describe('ModelSelector provider levels', () => {
517517
)).not.toBeNull();
518518
});
519519

520+
it('blocks sending with a missing pinned ID while keeping replacement models selectable', async () => {
521+
const onAvailabilityChange = vi.fn();
522+
flowChatStoreMocks.sessions.set('missing-model-session', {
523+
config: { agentType: 'Standard', modelName: 'removed-id' },
524+
});
525+
vi.mocked(configManager.getConfigs).mockResolvedValue({
526+
'ai.models': CATALOG_MODELS,
527+
'ai.default_models': { primary: 'acme-fast' },
528+
'ai.agent_model_defaults': { mode: 'primary' },
529+
});
530+
await act(async () => {
531+
root.render(<ModelSelector currentMode="Standard" sessionId="missing-model-session"
532+
onAvailabilityChange={onAvailabilityChange} />);
533+
await Promise.resolve();
534+
});
535+
expect(onAvailabilityChange).toHaveBeenLastCalledWith({
536+
status: 'target-model-unavailable', canSend: false,
537+
});
538+
await openMenu();
539+
await openProvider('provider-acme');
540+
expect(modelOption('acme-fast')).not.toBeNull();
541+
});
542+
520543
it('distinguishes configured models from enabled chat models', async () => {
521544
const onAvailabilityChange = vi.fn();
522545
vi.mocked(configManager.getConfigs).mockResolvedValue({

‎src/web-ui/src/flow_chat/services/flow-chat-manager/MessageModule.test.ts‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -980,6 +980,29 @@ describe('MessageModule detached dispatch', () => {
980980
});
981981

982982
describe('MessageModule model synchronization', () => {
983+
function modelSyncContext(modelName: string) {
984+
const session = {
985+
sessionId: 'same-name-session',
986+
config: { modelName, workspacePath: '/remote/repo' },
987+
remoteConnectionId: 'ssh-1', remoteSshHost: 'example.test',
988+
maxContextTokens: 32000,
989+
};
990+
const context: any = {
991+
flowChatStore: {
992+
getSurfaceGeneration: () => 0,
993+
getState: () => ({ sessions: new Map([[session.sessionId, session]]) }),
994+
updateSessionModelName: vi.fn(),
995+
updateSessionMaxContextTokens: vi.fn(),
996+
},
997+
};
998+
return { context, session };
999+
}
1000+
1001+
const sameNameModels = [
1002+
{ id: 'model-first', name: 'MOCK-8000', model_name: 'asdf', enabled: true, context_window: 32000 },
1003+
{ id: 'asdf', name: 'MOCK-8000-2', model_name: 'asdf', enabled: true, context_window: 64000 },
1004+
];
1005+
9831006
beforeEach(() => {
9841007
vi.clearAllMocks();
9851008
mockGetConfigs.mockResolvedValue({
@@ -993,6 +1016,31 @@ describe('MessageModule model synchronization', () => {
9931016
mockUpdateSessionModel.mockResolvedValue(undefined);
9941017
});
9951018

1019+
it('sends the exact selected account ID with the original remote routing', async () => {
1020+
mockGetConfigs.mockResolvedValue({
1021+
'ai.models': sameNameModels, 'ai.default_models': { primary: 'model-first' },
1022+
});
1023+
const { context, session } = modelSyncContext('asdf');
1024+
await syncSessionModelSelection(context, session.sessionId, 'Standard');
1025+
expect(context.flowChatStore.updateSessionModelName).not.toHaveBeenCalled();
1026+
expect(context.flowChatStore.updateSessionMaxContextTokens).toHaveBeenCalledWith(session.sessionId, 64000);
1027+
expect(mockUpdateSessionModel).toHaveBeenCalledWith(expect.objectContaining({
1028+
modelName: 'asdf', remoteConnectionId: 'ssh-1', remoteSshHost: 'example.test',
1029+
workspacePath: '/remote/repo',
1030+
}));
1031+
});
1032+
1033+
it.each(['MOCK-8000', 'asdf', 'removed-model'])('does not replace unavailable ID %s with a name match or Primary', async modelName => {
1034+
mockGetConfigs.mockResolvedValue({
1035+
'ai.models': [sameNameModels[0]], 'ai.default_models': { primary: 'model-first' },
1036+
});
1037+
const { context, session } = modelSyncContext(modelName);
1038+
await expect(syncSessionModelSelection(context, session.sessionId, 'Standard'))
1039+
.rejects.toThrow('model configuration ID');
1040+
expect(context.flowChatStore.updateSessionModelName).not.toHaveBeenCalled();
1041+
expect(mockUpdateSessionModel).not.toHaveBeenCalled();
1042+
});
1043+
9961044
it('keeps an explicit primary selector when synchronizing before send', async () => {
9971045
const session = {
9981046
sessionId: 'session-primary',

‎src/web-ui/src/flow_chat/utils/modelResolution.test.ts‎

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,13 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
66
import { aiApi } from '@/infrastructure/api/service-api/AIApi';
77
import { configManager } from '@/infrastructure/config/services/ConfigManager';
88
import { setRecentReasoningPreset } from './reasoningPresets';
9-
import { resolveReasoningPresetForSessionCreation } from './modelResolution';
9+
import {
10+
getModelMaxTokens,
11+
resolveModelReference,
12+
resolveModelSelection,
13+
resolveReasoningPresetForSessionCreation,
14+
} from './modelResolution';
15+
import type { AIModelConfig } from '@/infrastructure/config/types';
1016

1117
vi.mock('@/infrastructure/api/service-api/AIApi', () => ({
1218
aiApi: { getModelCatalog: vi.fn() },
@@ -16,6 +22,62 @@ vi.mock('@/infrastructure/config/services/ConfigManager', () => ({
1622
configManager: { getConfigs: vi.fn() },
1723
}));
1824

25+
describe('configured model identity', () => {
26+
const first: AIModelConfig = {
27+
id: 'model-first', name: 'MOCK-8000', model_name: 'asdf',
28+
provider: 'openai', base_url: 'https://example.test', enabled: true,
29+
category: 'general_chat', capabilities: ['text_chat'], context_window: 32000,
30+
metadata: { provider_instance_id: 'account-1' },
31+
};
32+
const selected: AIModelConfig = {
33+
...first, id: 'asdf', name: 'MOCK-8000-2', context_window: 64000,
34+
metadata: { provider_instance_id: 'account-2' },
35+
};
36+
const models = [first, selected];
37+
38+
it.each([models, [selected, first]])('preserves the selected account regardless of catalog order: %j', (...ordered) => {
39+
const result = resolveModelSelection({ models: ordered, sessionModelId: 'asdf' });
40+
expect(result).toMatchObject({
41+
model: selected, selectorId: 'asdf', concreteModelId: 'asdf',
42+
source: 'session', recovered: false,
43+
});
44+
});
45+
46+
it('does not match a display name even when it equals another config ID', () => {
47+
expect(resolveModelReference([{ ...first, name: 'asdf' }, selected], 'asdf')).toBe(selected);
48+
expect(resolveModelReference(models, 'MOCK-8000-2')).toBeNull();
49+
expect(resolveModelReference([first], 'asdf')).toBeNull();
50+
});
51+
52+
it.each(['primary', 'fast'])('resolves the %s alias through the exact configured ID', selector => {
53+
expect(resolveModelSelection({
54+
models, sessionModelId: selector, defaultModels: { primary: 'asdf', fast: 'asdf' },
55+
})).toMatchObject({ model: selected, selectorId: selector, concreteModelId: 'asdf' });
56+
});
57+
58+
it('keeps Fast fallback to the configured Primary ID', () => {
59+
expect(resolveModelReference(models, 'fast', { fast: 'missing', primary: 'asdf' })).toBe(selected);
60+
});
61+
62+
it.each(['missing', 'MOCK-8000-2'])('marks an unavailable pinned reference %s without choosing another account', sessionModelId => {
63+
expect(resolveModelSelection({ models, sessionModelId, defaultModels: { primary: 'model-first' } }))
64+
.toEqual({ model: null, source: 'session', recovered: true });
65+
});
66+
67+
it('rejects disabled, missing-ID and duplicate-ID entries', () => {
68+
expect(resolveModelReference([first, { ...selected, enabled: false }], 'asdf')).toBeNull();
69+
expect(resolveModelSelection({ models: [{ ...selected, id: undefined }] }).model).toBeNull();
70+
expect(resolveModelReference([selected, { ...first, id: 'asdf' }], 'asdf')).toBeNull();
71+
});
72+
73+
it('uses the selected account context window', async () => {
74+
vi.mocked(configManager.getConfigs).mockResolvedValue({
75+
'ai.models': models, 'ai.default_models': { primary: 'model-first' },
76+
});
77+
await expect(getModelMaxTokens('asdf')).resolves.toBe(64000);
78+
});
79+
});
80+
1981
describe('reasoning preset session creation resolution', () => {
2082
beforeEach(() => {
2183
const storage = new Map<string, string>();

‎src/web-ui/src/flow_chat/utils/modelResolution.ts‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,9 @@ export interface ModelSelectionResolution {
5050
function findSelectableModel(models: readonly AIModelConfig[], modelRef: string | null | undefined): AIModelConfig | null {
5151
const value = modelRef?.trim();
5252
if (!value) return null;
53-
return models.find(model => isSelectableTextChatModel(model)
54-
&& (model.id === value || model.name === value || model.model_name === value)
55-
) ?? null;
53+
// Config IDs identify credentials; display and upstream names do not.
54+
const matches = models.filter(model => isSelectableTextChatModel(model) && model.id === value);
55+
return matches.length === 1 ? matches[0] : null;
5656
}
5757

5858
function resolveModelForContextWindow(
@@ -135,20 +135,24 @@ export function resolveModelSelection({
135135
if (model) {
136136
const selectorId = ref === 'primary' || ref === 'fast'
137137
? ref
138-
: model.id?.trim() || model.model_name.trim();
138+
: model.id;
139139
return {
140140
model,
141141
selectorId,
142-
concreteModelId: model.id?.trim() || model.model_name.trim(),
142+
concreteModelId: model.id,
143143
source: candidate.source,
144144
recovered,
145145
};
146146
}
147+
// A pinned session must not silently switch accounts when its ID is unavailable.
148+
if (candidate.source === 'session') {
149+
return { model: null, source: 'session', recovered: true };
150+
}
147151
recovered = true;
148152
}
149153

150154
const fallback = selectableModels[0];
151-
const concreteModelId = fallback.id?.trim() || fallback.model_name.trim();
155+
const concreteModelId = fallback.id;
152156
return {
153157
model: fallback,
154158
selectorId: concreteModelId,

‎src/web-ui/src/flow_chat/utils/modelSync.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { configManager } from '@/infrastructure/config/services/ConfigManager';
1010
import type { AIModelConfig, AgentModelDefaultsConfig, DefaultModelsConfig } from '@/infrastructure/config/types';
1111
import { createLogger } from '@/shared/utils/logger';
1212
import type { FlowChatContext } from '../services/flow-chat-manager/types';
13-
import { getModelMaxTokens } from './modelResolution';
13+
import { getModelMaxTokens, resolveModelReference } from './modelResolution';
1414
import { sessionProjectWorkspacePath } from './sessionWorkspace';
1515
import {
1616
getActiveSurfaceScope,
@@ -34,11 +34,11 @@ function normalizeModelSelection(
3434
return matchedModel ? value : 'primary';
3535
}
3636

37-
const matchedModel = models.find(model =>
38-
model.enabled !== false
39-
&& (model.id === value || model.name === value || model.model_name === value),
40-
);
41-
return matchedModel?.id || 'primary';
37+
const matchedModel = resolveModelReference(models, value);
38+
if (!matchedModel?.id) {
39+
throw new Error(`Unknown, disabled, or ambiguous model configuration ID: ${value}`);
40+
}
41+
return matchedModel.id;
4242
}
4343

4444
export async function syncSessionModelSelection(

0 commit comments

Comments
 (0)