Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 73 additions & 1 deletion packages/cli/src/acp-integration/acpAgent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,20 @@ vi.mock('./runtimeOutputDirContext.js', () => ({
),
}));

vi.mock('./authMethods.js', () => ({ buildAuthMethods: vi.fn() }));
vi.mock('./authMethods.js', () => {
const buildAuthMethods = vi.fn();
return {
buildAuthMethods,
pickAuthMethodsForAuthRequired: vi.fn((selectedType?: string) => {
const authMethods = buildAuthMethods();
if (!selectedType) return authMethods;
const matched = authMethods.filter(
(method: { id: string }) => method.id === selectedType,
);
return matched.length ? matched : authMethods;
}),
};
});
vi.mock('./service/filesystem.js', () => ({
AcpFileSystemService: vi.fn(),
}));
Expand Down Expand Up @@ -195,6 +208,7 @@ import { loadSettings } from '../config/settings.js';
import { loadCliConfig } from '../config/config.js';
import { Session, buildAvailableCommandsSnapshot } from './session/Session.js';
import { SERVE_STATUS_EXT_METHODS } from '../serve/status.js';
import { buildAuthMethods } from './authMethods.js';

describe('runAcpAgent shutdown cleanup', () => {
let processExitSpy: MockInstance<typeof process.exit>;
Expand Down Expand Up @@ -792,6 +806,64 @@ describe('QwenAgent MCP SSE/HTTP support', () => {
await agentPromise;
});

it('does not return discontinued qwen-oauth as the only ACP auth option', async () => {
vi.mocked(buildAuthMethods).mockReturnValue([
{
id: 'openai',
name: 'Use OpenAI API key',
description: 'Requires setting OPENAI_API_KEY',
},
]);

const innerConfig = makeInnerConfig();
vi.mocked(innerConfig.getModelsConfig).mockReturnValue({
getCurrentAuthType: vi.fn().mockReturnValue('qwen-oauth'),
} as unknown as ReturnType<Config['getModelsConfig']>);
vi.mocked(innerConfig.refreshAuth).mockRejectedValue(
new Error('qwen-oauth token expired'),
);
vi.mocked(loadSettings).mockReturnValue(makeSessionSettings());
vi.mocked(loadCliConfig).mockResolvedValue(
innerConfig as unknown as Config,
);

vi.mocked(Session).mockImplementation(
() =>
({
getId: vi.fn().mockReturnValue('test-session-id'),
getConfig: vi.fn().mockReturnValue(innerConfig),
sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined),
replayHistory: vi.fn().mockResolvedValue(undefined),
installRewriter: vi.fn(),
}) as unknown as InstanceType<typeof Session>,
);

const agentPromise = runAcpAgent(
mockConfig,
makeSessionSettings(),
mockArgv,
);
await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined());
const agent = capturedAgentFactory!({
get closed() {
return mockConnectionState.promise;
},
}) as AgentLike;

await expect(
agent.newSession({ cwd: '/tmp', mcpServers: [] }),
).rejects.toMatchObject({
authMethods: [
expect.objectContaining({
id: 'openai',
}),
],
});

mockConnectionState.resolve();
await agentPromise;
});

function makeInnerConfig() {
return {
initialize: vi.fn().mockResolvedValue(undefined),
Expand Down
48 changes: 6 additions & 42 deletions packages/cli/src/acp-integration/acpAgent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import type { Content } from '@google/genai';
import type {
Agent,
AuthenticateRequest,
AuthMethod,
CancelNotification,
ClientCapabilities,
InitializeRequest,
Expand Down Expand Up @@ -69,7 +68,10 @@ import type {
SetSessionModeRequest,
SetSessionModeResponse,
} from '@agentclientprotocol/sdk';
import { buildAuthMethods } from './authMethods.js';
import {
buildAuthMethods,
pickAuthMethodsForAuthRequired,
} from './authMethods.js';
import { AcpFileSystemService } from './service/filesystem.js';
import { Readable, Writable } from 'node:stream';
import type { LoadedSettings } from '../config/settings.js';
Expand Down Expand Up @@ -1936,7 +1938,7 @@ class QwenAgent implements Agent {
const selectedType = config.getModelsConfig().getCurrentAuthType();
if (!selectedType) {
throw RequestError.authRequired(
{ authMethods: this.pickAuthMethodsForAuthRequired() },
{ authMethods: pickAuthMethodsForAuthRequired() },
'Use Qwen Code CLI to authenticate first.',
);
}
Expand All @@ -1947,51 +1949,13 @@ class QwenAgent implements Agent {
debugLogger.error(`Authentication failed: ${e}`);
throw RequestError.authRequired(
{
authMethods: this.pickAuthMethodsForAuthRequired(selectedType, e),
authMethods: pickAuthMethodsForAuthRequired(selectedType),
},
'Authentication failed: ' + (e as Error).message,
);
}
}

private pickAuthMethodsForAuthRequired(
selectedType?: AuthType | string,
error?: unknown,
): AuthMethod[] {
const authMethods = buildAuthMethods();
const errorMessage = this.extractErrorMessage(error);
if (
errorMessage?.includes('qwen-oauth') ||
errorMessage?.includes('Qwen OAuth')
) {
const qwenOAuthMethods = authMethods.filter(
(m) => m.id === AuthType.QWEN_OAUTH,
);
return qwenOAuthMethods.length ? qwenOAuthMethods : authMethods;
}

if (selectedType) {
const matched = authMethods.filter((m) => m.id === selectedType);
return matched.length ? matched : authMethods;
}

return authMethods;
}

private extractErrorMessage(error?: unknown): string | undefined {
if (error instanceof Error) return error.message;
if (
typeof error === 'object' &&
error != null &&
'message' in error &&
typeof error.message === 'string'
) {
return error.message;
}
if (typeof error === 'string') return error;
return undefined;
}

private setupFileSystem(config: Config): void {
if (!this.clientCapabilities?.fs) return;

Expand Down
15 changes: 14 additions & 1 deletion packages/cli/src/acp-integration/acpAgent.worktree.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,20 @@ vi.mock('./runtimeOutputDirContext.js', () => ({
),
}));

vi.mock('./authMethods.js', () => ({ buildAuthMethods: vi.fn() }));
vi.mock('./authMethods.js', () => {
const buildAuthMethods = vi.fn();
return {
buildAuthMethods,
pickAuthMethodsForAuthRequired: vi.fn((selectedType?: string) => {
const authMethods = buildAuthMethods();
if (!selectedType) return authMethods;
const matched = authMethods.filter(
(method: { id: string }) => method.id === selectedType,
);
return matched.length ? matched : authMethods;
}),
};
});
vi.mock('./service/filesystem.js', () => ({
AcpFileSystemService: vi.fn(),
}));
Expand Down
30 changes: 30 additions & 0 deletions packages/cli/src/acp-integration/authMethods.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/**
* @license
* Copyright 2025 Qwen Team
* SPDX-License-Identifier: Apache-2.0
*/

import { describe, expect, it } from 'vitest';
import { AuthType } from '@qwen-code/qwen-code-core';
import {
buildAuthMethods,
pickAuthMethodsForAuthRequired,
} from './authMethods.js';

describe('ACP auth methods', () => {
it('does not advertise discontinued Qwen OAuth', () => {
const authMethods = buildAuthMethods();

expect(authMethods.map((method) => method.id)).toEqual([
AuthType.USE_OPENAI,
]);
});

it('falls back to working methods for a stored discontinued Qwen OAuth selection', () => {
const authMethods = pickAuthMethodsForAuthRequired('qwen-oauth');

expect(authMethods.map((method) => method.id)).toEqual([
AuthType.USE_OPENAI,
]);
});
});
28 changes: 6 additions & 22 deletions packages/cli/src/acp-integration/authMethods.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,33 +18,17 @@ export function buildAuthMethods(): AuthMethod[] {
args: ['--auth-type=openai'],
},
},
{
id: AuthType.QWEN_OAUTH,
name: 'Qwen OAuth',
description: 'Qwen OAuth (free tier discontinued 2026-04-15)',
_meta: {
type: 'terminal',
args: ['--auth-type=qwen-oauth'],
},
},
];
}

export function filterAuthMethodsById(
authMethods: AuthMethod[],
authMethodId: string,
export function pickAuthMethodsForAuthRequired(
selectedType?: AuthType | string,
): AuthMethod[] {
return authMethods.filter((method) => method.id === authMethodId);
}

export function pickAuthMethodsForDetails(details?: string): AuthMethod[] {
const authMethods = buildAuthMethods();
if (!details) {
return authMethods;
}
if (details.includes('qwen-oauth') || details.includes('Qwen OAuth')) {
const narrowed = filterAuthMethodsById(authMethods, AuthType.QWEN_OAUTH);
return narrowed.length ? narrowed : authMethods;
if (selectedType) {
const matched = authMethods.filter((method) => method.id === selectedType);
return matched.length ? matched : authMethods;
}

return authMethods;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] After removing the qwen-oauth narrowing, pickAuthMethodsForDetails is now dead code — both branches return buildAuthMethods() unconditionally, and no production code calls it (only the new test file imports it). filterAuthMethodsById (lines 24–28) is similarly orphaned; its sole caller was the narrowing logic this PR removed.

Consider removing both functions to keep the module clean, along with the keeps working methods available when an error mentions Qwen OAuth test in authMethods.test.ts that exercises the now-trivial pickAuthMethodsForDetails.

Suggested change
return authMethods;

— qwen3.7-max via Qwen Code /review

}
Loading