mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-05 23:42:03 +00:00
Merge pull request #1622 from QwenLM/mingholy/fix/acp-auth-type
Fix: Use resolved authType to initialize ACP agent
This commit is contained in:
commit
93ed43c916
9 changed files with 75 additions and 68 deletions
|
|
@ -290,7 +290,7 @@ class GeminiAgent {
|
|||
}
|
||||
|
||||
private async ensureAuthenticated(config: Config): Promise<void> {
|
||||
const selectedType = this.settings.merged.security?.auth?.selectedType;
|
||||
const selectedType = config.getModelsConfig().getCurrentAuthType();
|
||||
if (!selectedType) {
|
||||
throw acp.RequestError.authRequired(
|
||||
'Use Qwen Code CLI to authenticate first.',
|
||||
|
|
|
|||
|
|
@ -168,7 +168,7 @@ describe('validateAuthMethod', () => {
|
|||
expect(validateAuthMethod(AuthType.USE_VERTEX_AI)).toBeNull();
|
||||
});
|
||||
|
||||
it('should use config.modelsConfig.getModel() when Config is provided', () => {
|
||||
it('should use config.getModelsConfig().getModel() when Config is provided', () => {
|
||||
// Settings has a different model
|
||||
vi.mocked(settings.loadSettings).mockReturnValue({
|
||||
merged: {
|
||||
|
|
@ -184,18 +184,18 @@ describe('validateAuthMethod', () => {
|
|||
|
||||
// Mock Config object that returns a different model (e.g., from CLI args)
|
||||
const mockConfig = {
|
||||
modelsConfig: {
|
||||
getModelsConfig: vi.fn().mockReturnValue({
|
||||
getModel: vi.fn().mockReturnValue('cli-model'),
|
||||
},
|
||||
}),
|
||||
} as unknown as import('@qwen-code/qwen-code-core').Config;
|
||||
|
||||
// Set the env key for the CLI model, not the settings model
|
||||
process.env['CLI_API_KEY'] = 'cli-key';
|
||||
|
||||
// Should use 'cli-model' from config.modelsConfig.getModel(), not 'settings-model'
|
||||
// Should use 'cli-model' from config.getModelsConfig().getModel(), not 'settings-model'
|
||||
const result = validateAuthMethod(AuthType.USE_OPENAI, mockConfig);
|
||||
expect(result).toBeNull();
|
||||
expect(mockConfig.modelsConfig.getModel).toHaveBeenCalled();
|
||||
expect(mockConfig.getModelsConfig).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should fail validation when Config provides different model without matching env key', () => {
|
||||
|
|
@ -217,9 +217,9 @@ describe('validateAuthMethod', () => {
|
|||
} as unknown as ReturnType<typeof settings.loadSettings>);
|
||||
|
||||
const mockConfig = {
|
||||
modelsConfig: {
|
||||
getModelsConfig: vi.fn().mockReturnValue({
|
||||
getModel: vi.fn().mockReturnValue('cli-model'),
|
||||
},
|
||||
}),
|
||||
} as unknown as import('@qwen-code/qwen-code-core').Config;
|
||||
|
||||
// Don't set CLI_API_KEY - validation should fail
|
||||
|
|
|
|||
|
|
@ -60,9 +60,9 @@ function hasApiKeyForAuth(
|
|||
| ModelProvidersConfig
|
||||
| undefined;
|
||||
|
||||
// Use config.modelsConfig.getModel() if available for accurate model ID resolution
|
||||
// Use config.getModelsConfig().getModel() if available for accurate model ID resolution
|
||||
// that accounts for CLI args, env vars, and settings. Fall back to settings.model.name.
|
||||
const modelId = config?.modelsConfig.getModel() ?? settings.model?.name;
|
||||
const modelId = config?.getModelsConfig().getModel() ?? settings.model?.name;
|
||||
|
||||
// Try to find model-specific envKey from modelProviders
|
||||
const modelConfig = findModelConfig(modelProviders, authType, modelId);
|
||||
|
|
@ -184,9 +184,9 @@ export function validateAuthMethod(
|
|||
const modelProviders = settings.merged.modelProviders as
|
||||
| ModelProvidersConfig
|
||||
| undefined;
|
||||
// Use config.modelsConfig.getModel() if available for accurate model ID
|
||||
// Use config.getModelsConfig().getModel() if available for accurate model ID
|
||||
const modelId =
|
||||
config?.modelsConfig.getModel() ?? settings.merged.model?.name;
|
||||
config?.getModelsConfig().getModel() ?? settings.merged.model?.name;
|
||||
const modelConfig = findModelConfig(modelProviders, authMethod, modelId);
|
||||
|
||||
if (modelConfig && !modelConfig.baseUrl) {
|
||||
|
|
|
|||
|
|
@ -47,7 +47,7 @@ export async function initializeApp(
|
|||
|
||||
// Use authType from modelsConfig which respects CLI --auth-type argument
|
||||
// over settings.security.auth.selectedType
|
||||
const authType = config.modelsConfig.getCurrentAuthType();
|
||||
const authType = config.getModelsConfig().getCurrentAuthType();
|
||||
const authError = await performInitialAuth(config, authType);
|
||||
|
||||
// Fallback to user select when initial authentication fails
|
||||
|
|
@ -61,7 +61,7 @@ export async function initializeApp(
|
|||
const themeError = validateTheme(settings);
|
||||
|
||||
const shouldOpenAuthDialog =
|
||||
!config.modelsConfig.wasAuthTypeExplicitlyProvided() || !!authError;
|
||||
!config.getModelsConfig().wasAuthTypeExplicitlyProvided() || !!authError;
|
||||
|
||||
if (config.getIdeMode()) {
|
||||
const ideClient = await IdeClient.getInstance();
|
||||
|
|
|
|||
|
|
@ -252,7 +252,7 @@ export async function main() {
|
|||
if (!settings.merged.security?.auth?.useExternal) {
|
||||
// Validate authentication here because the sandbox will interfere with the Oauth2 web redirect.
|
||||
try {
|
||||
const authType = partialConfig.modelsConfig.getCurrentAuthType();
|
||||
const authType = partialConfig.getModelsConfig().getCurrentAuthType();
|
||||
// Fresh users may not have selected/persisted an authType yet.
|
||||
// In that case, defer auth prompting/selection to the main interactive flow.
|
||||
if (authType) {
|
||||
|
|
|
|||
|
|
@ -434,7 +434,7 @@ export const AppContainer = (props: AppContainerProps) => {
|
|||
// Check for enforced auth type mismatch
|
||||
useEffect(() => {
|
||||
// Check for initialization error first
|
||||
const currentAuthType = config.modelsConfig.getCurrentAuthType();
|
||||
const currentAuthType = config.getModelsConfig().getCurrentAuthType();
|
||||
|
||||
if (
|
||||
settings.merged.security?.auth?.enforcedType &&
|
||||
|
|
|
|||
|
|
@ -14,18 +14,24 @@ import * as JsonOutputAdapterModule from './nonInteractive/io/JsonOutputAdapter.
|
|||
import * as StreamJsonOutputAdapterModule from './nonInteractive/io/StreamJsonOutputAdapter.js';
|
||||
import * as cleanupModule from './utils/cleanup.js';
|
||||
|
||||
type ModelsConfig = ReturnType<Config['getModelsConfig']>;
|
||||
|
||||
// Helper to create a mock Config with modelsConfig
|
||||
function createMockConfig(overrides?: Partial<Config>): Config {
|
||||
return {
|
||||
const baseModelsConfig = {
|
||||
getModel: vi.fn().mockReturnValue('default-model'),
|
||||
getCurrentAuthType: vi.fn().mockReturnValue(AuthType.QWEN_OAUTH),
|
||||
} as unknown as ModelsConfig;
|
||||
const baseConfig: Partial<Config> = {
|
||||
refreshAuth: vi.fn().mockResolvedValue('refreshed'),
|
||||
getOutputFormat: vi.fn().mockReturnValue(OutputFormat.TEXT),
|
||||
getContentGeneratorConfig: vi.fn().mockReturnValue({ authType: undefined }),
|
||||
modelsConfig: {
|
||||
getModel: vi.fn().mockReturnValue('default-model'),
|
||||
getCurrentAuthType: vi.fn().mockReturnValue(AuthType.QWEN_OAUTH),
|
||||
},
|
||||
getModelsConfig: vi.fn().mockReturnValue(baseModelsConfig),
|
||||
};
|
||||
return {
|
||||
...baseConfig,
|
||||
...overrides,
|
||||
} as unknown as Config;
|
||||
} as Config;
|
||||
}
|
||||
|
||||
describe('validateNonInterActiveAuth', () => {
|
||||
|
|
@ -128,10 +134,10 @@ describe('validateNonInterActiveAuth', () => {
|
|||
);
|
||||
const nonInteractiveConfig = createMockConfig({
|
||||
refreshAuth: refreshAuthMock,
|
||||
modelsConfig: {
|
||||
getModelsConfig: vi.fn().mockReturnValue({
|
||||
getModel: vi.fn().mockReturnValue('default-model'),
|
||||
getCurrentAuthType: vi.fn().mockReturnValue(AuthType.QWEN_OAUTH),
|
||||
},
|
||||
}),
|
||||
});
|
||||
try {
|
||||
await validateNonInteractiveAuth(
|
||||
|
|
@ -153,10 +159,10 @@ describe('validateNonInterActiveAuth', () => {
|
|||
process.env['OPENAI_API_KEY'] = 'fake-openai-key';
|
||||
const nonInteractiveConfig = createMockConfig({
|
||||
refreshAuth: refreshAuthMock,
|
||||
modelsConfig: {
|
||||
getModelsConfig: vi.fn().mockReturnValue({
|
||||
getModel: vi.fn().mockReturnValue('default-model'),
|
||||
getCurrentAuthType: vi.fn().mockReturnValue(AuthType.USE_OPENAI),
|
||||
},
|
||||
}),
|
||||
});
|
||||
await validateNonInteractiveAuth(
|
||||
undefined,
|
||||
|
|
@ -169,10 +175,10 @@ describe('validateNonInterActiveAuth', () => {
|
|||
it('uses configured QWEN_OAUTH if provided', async () => {
|
||||
const nonInteractiveConfig = createMockConfig({
|
||||
refreshAuth: refreshAuthMock,
|
||||
modelsConfig: {
|
||||
getModelsConfig: vi.fn().mockReturnValue({
|
||||
getModel: vi.fn().mockReturnValue('default-model'),
|
||||
getCurrentAuthType: vi.fn().mockReturnValue(AuthType.QWEN_OAUTH),
|
||||
},
|
||||
}),
|
||||
});
|
||||
await validateNonInteractiveAuth(
|
||||
undefined,
|
||||
|
|
@ -222,7 +228,7 @@ describe('validateNonInterActiveAuth', () => {
|
|||
expect(validateAuthMethodSpy).not.toHaveBeenCalled();
|
||||
expect(consoleErrorSpy).not.toHaveBeenCalled();
|
||||
expect(processExitSpy).not.toHaveBeenCalled();
|
||||
// refreshAuth is called with the authType from config.modelsConfig.getCurrentAuthType()
|
||||
// refreshAuth is called with the authType from config.getModelsConfig().getCurrentAuthType()
|
||||
expect(refreshAuthMock).toHaveBeenCalledWith(AuthType.QWEN_OAUTH);
|
||||
});
|
||||
|
||||
|
|
@ -233,10 +239,10 @@ describe('validateNonInterActiveAuth', () => {
|
|||
process.env['OPENAI_API_KEY'] = 'fake-key';
|
||||
const nonInteractiveConfig = createMockConfig({
|
||||
refreshAuth: refreshAuthMock,
|
||||
modelsConfig: {
|
||||
getModelsConfig: vi.fn().mockReturnValue({
|
||||
getModel: vi.fn().mockReturnValue('default-model'),
|
||||
getCurrentAuthType: vi.fn().mockReturnValue(AuthType.USE_OPENAI),
|
||||
},
|
||||
}),
|
||||
});
|
||||
await validateNonInteractiveAuth(
|
||||
undefined,
|
||||
|
|
@ -251,10 +257,10 @@ describe('validateNonInterActiveAuth', () => {
|
|||
process.env['OPENAI_API_KEY'] = 'fake-key';
|
||||
const nonInteractiveConfig = createMockConfig({
|
||||
refreshAuth: refreshAuthMock,
|
||||
modelsConfig: {
|
||||
getModelsConfig: vi.fn().mockReturnValue({
|
||||
getModel: vi.fn().mockReturnValue('default-model'),
|
||||
getCurrentAuthType: vi.fn().mockReturnValue(AuthType.USE_OPENAI),
|
||||
},
|
||||
}),
|
||||
});
|
||||
try {
|
||||
await validateNonInteractiveAuth(
|
||||
|
|
@ -297,10 +303,10 @@ describe('validateNonInterActiveAuth', () => {
|
|||
const nonInteractiveConfig = createMockConfig({
|
||||
refreshAuth: refreshAuthMock,
|
||||
getOutputFormat: vi.fn().mockReturnValue(OutputFormat.JSON),
|
||||
modelsConfig: {
|
||||
getModelsConfig: vi.fn().mockReturnValue({
|
||||
getModel: vi.fn().mockReturnValue('default-model'),
|
||||
getCurrentAuthType: vi.fn().mockReturnValue(AuthType.QWEN_OAUTH),
|
||||
},
|
||||
}),
|
||||
});
|
||||
|
||||
try {
|
||||
|
|
@ -334,10 +340,10 @@ describe('validateNonInterActiveAuth', () => {
|
|||
const nonInteractiveConfig = createMockConfig({
|
||||
refreshAuth: refreshAuthMock,
|
||||
getOutputFormat: vi.fn().mockReturnValue(OutputFormat.JSON),
|
||||
modelsConfig: {
|
||||
getModelsConfig: vi.fn().mockReturnValue({
|
||||
getModel: vi.fn().mockReturnValue('default-model'),
|
||||
getCurrentAuthType: vi.fn().mockReturnValue(AuthType.USE_OPENAI),
|
||||
},
|
||||
}),
|
||||
});
|
||||
|
||||
try {
|
||||
|
|
@ -373,10 +379,10 @@ describe('validateNonInterActiveAuth', () => {
|
|||
const nonInteractiveConfig = createMockConfig({
|
||||
refreshAuth: refreshAuthMock,
|
||||
getOutputFormat: vi.fn().mockReturnValue(OutputFormat.JSON),
|
||||
modelsConfig: {
|
||||
getModelsConfig: vi.fn().mockReturnValue({
|
||||
getModel: vi.fn().mockReturnValue('default-model'),
|
||||
getCurrentAuthType: vi.fn().mockReturnValue(AuthType.USE_OPENAI),
|
||||
},
|
||||
}),
|
||||
});
|
||||
|
||||
try {
|
||||
|
|
@ -433,10 +439,10 @@ describe('validateNonInterActiveAuth', () => {
|
|||
refreshAuth: refreshAuthMock,
|
||||
getOutputFormat: vi.fn().mockReturnValue(OutputFormat.STREAM_JSON),
|
||||
getIncludePartialMessages: vi.fn().mockReturnValue(false),
|
||||
modelsConfig: {
|
||||
getModelsConfig: vi.fn().mockReturnValue({
|
||||
getModel: vi.fn().mockReturnValue('default-model'),
|
||||
getCurrentAuthType: vi.fn().mockReturnValue(AuthType.QWEN_OAUTH),
|
||||
},
|
||||
}),
|
||||
});
|
||||
|
||||
try {
|
||||
|
|
@ -471,10 +477,10 @@ describe('validateNonInterActiveAuth', () => {
|
|||
refreshAuth: refreshAuthMock,
|
||||
getOutputFormat: vi.fn().mockReturnValue(OutputFormat.STREAM_JSON),
|
||||
getIncludePartialMessages: vi.fn().mockReturnValue(false),
|
||||
modelsConfig: {
|
||||
getModelsConfig: vi.fn().mockReturnValue({
|
||||
getModel: vi.fn().mockReturnValue('default-model'),
|
||||
getCurrentAuthType: vi.fn().mockReturnValue(AuthType.USE_OPENAI),
|
||||
},
|
||||
}),
|
||||
});
|
||||
|
||||
try {
|
||||
|
|
@ -511,10 +517,10 @@ describe('validateNonInterActiveAuth', () => {
|
|||
refreshAuth: refreshAuthMock,
|
||||
getOutputFormat: vi.fn().mockReturnValue(OutputFormat.STREAM_JSON),
|
||||
getIncludePartialMessages: vi.fn().mockReturnValue(false),
|
||||
modelsConfig: {
|
||||
getModelsConfig: vi.fn().mockReturnValue({
|
||||
getModel: vi.fn().mockReturnValue('default-model'),
|
||||
getCurrentAuthType: vi.fn().mockReturnValue(AuthType.USE_OPENAI),
|
||||
},
|
||||
}),
|
||||
});
|
||||
|
||||
try {
|
||||
|
|
|
|||
|
|
@ -19,7 +19,9 @@ export async function validateNonInteractiveAuth(
|
|||
): Promise<Config> {
|
||||
try {
|
||||
// Get the actual authType from config which has already resolved CLI args, env vars, and settings
|
||||
const authType = nonInteractiveConfig.modelsConfig.getCurrentAuthType();
|
||||
const authType = nonInteractiveConfig
|
||||
.getModelsConfig()
|
||||
.getCurrentAuthType();
|
||||
if (!authType) {
|
||||
throw new Error(
|
||||
'No auth type is selected. Please configure an auth type (e.g. via settings or `--auth-type`) before running in non-interactive mode.',
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue