Skip to content

Commit 14826dd

Browse files
MingholyMingholy
committed
fix: provide available models of all configured authTypes
1 parent 6eb16c0 commit 14826dd

10 files changed

Lines changed: 347 additions & 77 deletions

File tree

‎packages/cli/src/acp-integration/acpAgent.ts‎

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ import { ExtensionEnablementManager } from '../config/extensions/extensionEnable
3434

3535
// Import the modular Session class
3636
import { Session } from './session/Session.js';
37+
import {
38+
formatAcpModelId,
39+
parseAcpBaseModelId,
40+
} from '../utils/acpModelUtils.js';
3741

3842
export async function runAcpAgent(
3943
config: Config,
@@ -381,15 +385,24 @@ class GeminiAgent {
381385
private buildAvailableModels(
382386
config: Config,
383387
): acp.NewSessionResponse['models'] {
384-
const currentModelId = (
388+
const rawCurrentModelId = (
385389
config.getModel() ||
386390
this.config.getModel() ||
387391
''
388392
).trim();
389-
const availableModels = config.getAvailableModels();
393+
const currentAuthType = config.getAuthType();
394+
const allConfiguredModels = config.getAllConfiguredModels();
395+
396+
const baseCurrentModelId = parseAcpBaseModelId(rawCurrentModelId);
397+
const currentModelId =
398+
currentAuthType && baseCurrentModelId
399+
? formatAcpModelId(baseCurrentModelId, currentAuthType)
400+
: baseCurrentModelId;
401+
402+
const availableModels = allConfiguredModels;
390403

391404
const mappedAvailableModels = availableModels.map((model) => ({
392-
modelId: model.id,
405+
modelId: formatAcpModelId(model.id, model.authType),
393406
name: model.label,
394407
description: model.description ?? null,
395408
_meta: {
@@ -406,7 +419,7 @@ class GeminiAgent {
406419
name: currentModelId,
407420
description: null,
408421
_meta: {
409-
contextLimit: tokenLimit(currentModelId),
422+
contextLimit: tokenLimit(baseCurrentModelId),
410423
},
411424
});
412425
}

‎packages/cli/src/acp-integration/session/Session.test.ts‎

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import { describe, it, expect, vi, beforeEach } from 'vitest';
88
import { Session } from './Session.js';
99
import type { Config, GeminiChat } from '@qwen-code/qwen-code-core';
10-
import { ApprovalMode } from '@qwen-code/qwen-code-core';
10+
import { ApprovalMode, AuthType } from '@qwen-code/qwen-code-core';
1111
import type * as acp from '../acp.js';
1212
import type { LoadedSettings } from '../../config/settings.js';
1313
import * as nonInteractiveCliCommands from '../../nonInteractiveCliCommands.js';
@@ -24,14 +24,19 @@ describe('Session', () => {
2424
let mockSettings: LoadedSettings;
2525
let session: Session;
2626
let currentModel: string;
27-
let setModelSpy: ReturnType<typeof vi.fn>;
27+
let currentAuthType: AuthType;
28+
let switchModelSpy: ReturnType<typeof vi.fn>;
2829
let getAvailableCommandsSpy: ReturnType<typeof vi.fn>;
2930

3031
beforeEach(() => {
3132
currentModel = 'qwen3-code-plus';
32-
setModelSpy = vi.fn().mockImplementation(async (modelId: string) => {
33-
currentModel = modelId;
34-
});
33+
currentAuthType = AuthType.USE_OPENAI;
34+
switchModelSpy = vi
35+
.fn()
36+
.mockImplementation(async (authType: AuthType, modelId: string) => {
37+
currentAuthType = authType;
38+
currentModel = modelId;
39+
});
3540

3641
mockChat = {
3742
sendMessageStream: vi.fn(),
@@ -40,8 +45,9 @@ describe('Session', () => {
4045

4146
mockConfig = {
4247
setApprovalMode: vi.fn(),
43-
setModel: setModelSpy,
48+
switchModel: switchModelSpy,
4449
getModel: vi.fn().mockImplementation(() => currentModel),
50+
getAuthType: vi.fn().mockImplementation(() => currentAuthType),
4551
} as unknown as Config;
4652

4753
mockClient = {
@@ -88,17 +94,25 @@ describe('Session', () => {
8894

8995
describe('setModel', () => {
9096
it('sets model via config and returns current model', async () => {
97+
const requested = `qwen3-coder-plus(${AuthType.USE_OPENAI})`;
9198
const result = await session.setModel({
9299
sessionId: 'test-session-id',
93-
modelId: ' qwen3-coder-plus ',
100+
modelId: ` ${requested} `,
94101
});
95102

96-
expect(mockConfig.setModel).toHaveBeenCalledWith('qwen3-coder-plus', {
97-
reason: 'user_request_acp',
98-
context: 'session/set_model',
99-
});
103+
expect(mockConfig.switchModel).toHaveBeenCalledWith(
104+
AuthType.USE_OPENAI,
105+
'qwen3-coder-plus',
106+
undefined,
107+
{
108+
reason: 'user_request_acp',
109+
context: 'session/set_model',
110+
},
111+
);
100112
expect(mockConfig.getModel).toHaveBeenCalled();
101-
expect(result).toEqual({ modelId: 'qwen3-coder-plus' });
113+
expect(result).toEqual({
114+
modelId: `qwen3-coder-plus(${AuthType.USE_OPENAI})`,
115+
});
102116
});
103117

104118
it('rejects empty/whitespace model IDs', async () => {
@@ -109,17 +123,17 @@ describe('Session', () => {
109123
}),
110124
).rejects.toThrow('Invalid params');
111125

112-
expect(mockConfig.setModel).not.toHaveBeenCalled();
126+
expect(mockConfig.switchModel).not.toHaveBeenCalled();
113127
});
114128

115-
it('propagates errors from config.setModel', async () => {
129+
it('propagates errors from config.switchModel', async () => {
116130
const configError = new Error('Invalid model');
117-
setModelSpy.mockRejectedValueOnce(configError);
131+
switchModelSpy.mockRejectedValueOnce(configError);
118132

119133
await expect(
120134
session.setModel({
121135
sessionId: 'test-session-id',
122-
modelId: 'invalid-model',
136+
modelId: `invalid-model(${AuthType.USE_OPENAI})`,
123137
}),
124138
).rejects.toThrow('Invalid model');
125139
});

‎packages/cli/src/acp-integration/session/Session.ts‎

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import type {
1919
SubAgentEventEmitter,
2020
} from '@qwen-code/qwen-code-core';
2121
import {
22+
AuthType,
2223
ApprovalMode,
2324
convertToFunctionResponse,
2425
DiscoveredMCPTool,
@@ -58,6 +59,10 @@ import type {
5859
CurrentModeUpdate,
5960
} from '../schema.js';
6061
import { isSlashCommand } from '../../ui/utils/commandUtils.js';
62+
import {
63+
formatAcpModelId,
64+
parseAcpModelOption,
65+
} from '../../utils/acpModelUtils.js';
6166

6267
// Import modular session components
6368
import type { SessionContext, ToolCallStartParams } from './types.js';
@@ -355,23 +360,39 @@ export class Session implements SessionContext {
355360
* Validates the model ID and switches the model via Config.
356361
*/
357362
async setModel(params: SetModelRequest): Promise<SetModelResponse> {
358-
const modelId = params.modelId.trim();
363+
const rawModelId = params.modelId.trim();
359364

360-
if (!modelId) {
365+
if (!rawModelId) {
361366
throw acp.RequestError.invalidParams('modelId cannot be empty');
362367
}
363368

364-
// Attempt to set the model using config
365-
await this.config.setModel(modelId, {
366-
reason: 'user_request_acp',
367-
context: 'session/set_model',
368-
});
369+
const parsed = parseAcpModelOption(rawModelId);
370+
const previousAuthType = this.config.getAuthType?.();
371+
const selectedAuthType = parsed.authType ?? previousAuthType;
372+
373+
if (!selectedAuthType) {
374+
throw acp.RequestError.invalidParams('authType cannot be determined');
375+
}
376+
377+
await this.config.switchModel(
378+
selectedAuthType,
379+
parsed.modelId,
380+
selectedAuthType !== previousAuthType &&
381+
selectedAuthType === AuthType.QWEN_OAUTH
382+
? { requireCachedCredentials: true }
383+
: undefined,
384+
{
385+
reason: 'user_request_acp',
386+
context: 'session/set_model',
387+
},
388+
);
369389

370390
// Get updated model info
371391
const currentModel = this.config.getModel();
392+
const currentAuthType = this.config.getAuthType?.() ?? selectedAuthType;
372393

373394
return {
374-
modelId: currentModel,
395+
modelId: formatAcpModelId(currentModel, currentAuthType),
375396
};
376397
}
377398

‎packages/cli/src/ui/components/ModelDialog.test.tsx‎

Lines changed: 46 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -47,30 +47,36 @@ const renderComponent = (
4747
setValue: vi.fn(),
4848
} as unknown as LoadedSettings;
4949

50-
const mockConfig = contextValue
51-
? ({
52-
// --- Functions used by ModelDialog ---
53-
getModel: vi.fn(() => MAINLINE_CODER),
54-
setModel: vi.fn().mockResolvedValue(undefined),
55-
switchModel: vi.fn().mockResolvedValue(undefined),
56-
getAuthType: vi.fn(() => 'qwen-oauth'),
57-
58-
// --- Functions used by ClearcutLogger ---
59-
getUsageStatisticsEnabled: vi.fn(() => true),
60-
getSessionId: vi.fn(() => 'mock-session-id'),
61-
getDebugMode: vi.fn(() => false),
62-
getContentGeneratorConfig: vi.fn(() => ({
63-
authType: AuthType.QWEN_OAUTH,
64-
model: MAINLINE_CODER,
65-
})),
66-
getUseSmartEdit: vi.fn(() => false),
67-
getUseModelRouter: vi.fn(() => false),
68-
getProxy: vi.fn(() => undefined),
69-
70-
// --- Spread test-specific overrides ---
71-
...contextValue,
72-
} as unknown as Config)
73-
: undefined;
50+
const mockConfig = {
51+
// --- Functions used by ModelDialog ---
52+
getModel: vi.fn(() => MAINLINE_CODER),
53+
setModel: vi.fn().mockResolvedValue(undefined),
54+
switchModel: vi.fn().mockResolvedValue(undefined),
55+
getAuthType: vi.fn(() => 'qwen-oauth'),
56+
getAllConfiguredModels: vi.fn(() =>
57+
AVAILABLE_MODELS_QWEN.map((m) => ({
58+
id: m.id,
59+
label: m.label,
60+
description: m.description || '',
61+
authType: AuthType.QWEN_OAUTH,
62+
})),
63+
),
64+
65+
// --- Functions used by ClearcutLogger ---
66+
getUsageStatisticsEnabled: vi.fn(() => true),
67+
getSessionId: vi.fn(() => 'mock-session-id'),
68+
getDebugMode: vi.fn(() => false),
69+
getContentGeneratorConfig: vi.fn(() => ({
70+
authType: AuthType.QWEN_OAUTH,
71+
model: MAINLINE_CODER,
72+
})),
73+
getUseSmartEdit: vi.fn(() => false),
74+
getUseModelRouter: vi.fn(() => false),
75+
getProxy: vi.fn(() => undefined),
76+
77+
// --- Spread test-specific overrides ---
78+
...(contextValue ?? {}),
79+
} as unknown as Config;
7480

7581
const renderResult = render(
7682
<SettingsContext.Provider value={mockSettings}>
@@ -308,6 +314,14 @@ describe('<ModelDialog />', () => {
308314
{
309315
getModel: mockGetModel,
310316
getAuthType: mockGetAuthType,
317+
getAllConfiguredModels: vi.fn(() =>
318+
AVAILABLE_MODELS_QWEN.map((m) => ({
319+
id: m.id,
320+
label: m.label,
321+
description: m.description || '',
322+
authType: AuthType.QWEN_OAUTH,
323+
})),
324+
),
311325
} as unknown as Config
312326
}
313327
>
@@ -322,6 +336,14 @@ describe('<ModelDialog />', () => {
322336
const newMockConfig = {
323337
getModel: mockGetModel,
324338
getAuthType: mockGetAuthType,
339+
getAllConfiguredModels: vi.fn(() =>
340+
AVAILABLE_MODELS_QWEN.map((m) => ({
341+
id: m.id,
342+
label: m.label,
343+
description: m.description || '',
344+
authType: AuthType.QWEN_OAUTH,
345+
})),
346+
),
325347
} as unknown as Config;
326348

327349
rerender(

‎packages/cli/src/ui/components/ModelDialog.tsx‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
AuthType,
1212
ModelSlashCommandEvent,
1313
logModelSlashCommand,
14+
type AvailableModel as CoreAvailableModel,
1415
type ContentGeneratorConfig,
1516
type ContentGeneratorConfigSource,
1617
type ContentGeneratorConfigSources,
@@ -21,10 +22,7 @@ import { DescriptiveRadioButtonSelect } from './shared/DescriptiveRadioButtonSel
2122
import { ConfigContext } from '../contexts/ConfigContext.js';
2223
import { UIStateContext } from '../contexts/UIStateContext.js';
2324
import { useSettings } from '../contexts/SettingsContext.js';
24-
import {
25-
getAvailableModelsForAuthType,
26-
MAINLINE_CODER,
27-
} from '../models/availableModels.js';
25+
import { MAINLINE_CODER } from '../models/availableModels.js';
2826
import { getPersistScopeForModelSelection } from '../../config/modelProvidersScope.js';
2927
import { t } from '../../i18n/index.js';
3028

@@ -154,13 +152,17 @@ export function ModelDialog({ onClose }: ModelDialogProps): React.JSX.Element {
154152
const sources = readSourcesFromConfig(config);
155153

156154
const availableModelEntries = useMemo(() => {
157-
const allAuthTypes = Object.values(AuthType) as AuthType[];
158-
const modelsByAuthType = allAuthTypes
159-
.map((t) => ({
160-
authType: t,
161-
models: getAvailableModelsForAuthType(t, config ?? undefined),
162-
}))
163-
.filter((x) => x.models.length > 0);
155+
const allModels = config ? config.getAllConfiguredModels() : [];
156+
157+
// Group models by authType
158+
const modelsByAuthTypeMap = new Map<AuthType, CoreAvailableModel[]>();
159+
for (const model of allModels) {
160+
const authType = model.authType;
161+
if (!modelsByAuthTypeMap.has(authType)) {
162+
modelsByAuthTypeMap.set(authType, []);
163+
}
164+
modelsByAuthTypeMap.get(authType)!.push(model);
165+
}
164166

165167
// Fixed order: qwen-oauth first, then others in a stable order
166168
const authTypeOrder: AuthType[] = [
@@ -171,15 +173,14 @@ export function ModelDialog({ onClose }: ModelDialogProps): React.JSX.Element {
171173
AuthType.USE_VERTEX_AI,
172174
];
173175

174-
// Filter to only include authTypes that have models
175-
const availableAuthTypes = new Set(modelsByAuthType.map((x) => x.authType));
176+
// Filter to only include authTypes that have models and maintain order
177+
const availableAuthTypes = new Set(modelsByAuthTypeMap.keys());
176178
const orderedAuthTypes = authTypeOrder.filter((t) =>
177179
availableAuthTypes.has(t),
178180
);
179181

180182
return orderedAuthTypes.flatMap((t) => {
181-
const models =
182-
modelsByAuthType.find((x) => x.authType === t)?.models ?? [];
183+
const models = modelsByAuthTypeMap.get(t) ?? [];
183184
return models.map((m) => ({ authType: t, model: m }));
184185
});
185186
}, [config]);

0 commit comments

Comments
 (0)