diff --git a/packages/cli/src/acp-integration/acp.ts b/packages/cli/src/acp-integration/acp.ts index 2ef78bbd..84ba5ff5 100644 --- a/packages/cli/src/acp-integration/acp.ts +++ b/packages/cli/src/acp-integration/acp.ts @@ -88,6 +88,16 @@ export class AgentSideConnection implements Client { ); } + /** + * Streams authentication updates (e.g. Qwen OAuth authUri) to the client. + */ + async authenticateUpdate(params: schema.AuthenticateUpdate): Promise { + return await this.#connection.sendNotification( + schema.CLIENT_METHODS.authenticate_update, + params, + ); + } + /** * Request permission before running a tool * @@ -241,9 +251,11 @@ class Connection { ).toResult(); } + let errorName; let details; if (error instanceof Error) { + errorName = error.name; details = error.message; } else if ( typeof error === 'object' && @@ -254,6 +266,10 @@ class Connection { details = error.message; } + if (errorName === 'TokenManagerError') { + return RequestError.authRequired(details).toResult(); + } + return RequestError.internalError(details).toResult(); } } @@ -357,6 +373,7 @@ export interface Client { params: schema.RequestPermissionRequest, ): Promise; sessionUpdate(params: schema.SessionNotification): Promise; + authenticateUpdate(params: schema.AuthenticateUpdate): Promise; writeTextFile( params: schema.WriteTextFileRequest, ): Promise; diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index fc3c4ccc..53cb8b1c 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -6,15 +6,19 @@ import type { ReadableStream, WritableStream } from 'node:stream/web'; -import type { Config, ConversationRecord } from '@qwen-code/qwen-code-core'; import { APPROVAL_MODE_INFO, APPROVAL_MODES, AuthType, clearCachedCredentialFile, + QwenOAuth2Event, + qwenOAuth2Events, MCPServerConfig, SessionService, buildApiHistoryFromConversation, + type Config, + type ConversationRecord, + type DeviceAuthorizationData, } from '@qwen-code/qwen-code-core'; import type { ApprovalModeValue } from './schema.js'; import * as acp from './acp.js'; @@ -123,13 +127,33 @@ class GeminiAgent { async authenticate({ methodId }: acp.AuthenticateRequest): Promise { const method = z.nativeEnum(AuthType).parse(methodId); + let authUri: string | undefined; + const authUriHandler = (deviceAuth: DeviceAuthorizationData) => { + authUri = deviceAuth.verification_uri_complete; + // Send the auth URL to ACP client as soon as it's available (refreshAuth is blocking). + void this.client.authenticateUpdate({ _meta: { authUri } }); + }; + + if (method === AuthType.QWEN_OAUTH) { + qwenOAuth2Events.once(QwenOAuth2Event.AuthUri, authUriHandler); + } + await clearCachedCredentialFile(); - await this.config.refreshAuth(method); - this.settings.setValue( - SettingScope.User, - 'security.auth.selectedType', - method, - ); + try { + await this.config.refreshAuth(method); + this.settings.setValue( + SettingScope.User, + 'security.auth.selectedType', + method, + ); + } finally { + // Ensure we don't leak listeners if auth fails early. + if (method === AuthType.QWEN_OAUTH) { + qwenOAuth2Events.off(QwenOAuth2Event.AuthUri, authUriHandler); + } + } + + return; } async newSession({ @@ -272,7 +296,8 @@ class GeminiAgent { } try { - await config.refreshAuth(selectedType); + // Use true for the second argument to ensure only cached credentials are used + await config.refreshAuth(selectedType, true); } catch (e) { console.error(`Authentication failed: ${e}`); throw acp.RequestError.authRequired(); diff --git a/packages/cli/src/acp-integration/schema.ts b/packages/cli/src/acp-integration/schema.ts index ac754318..a557c519 100644 --- a/packages/cli/src/acp-integration/schema.ts +++ b/packages/cli/src/acp-integration/schema.ts @@ -20,6 +20,7 @@ export const AGENT_METHODS = { export const CLIENT_METHODS = { fs_read_text_file: 'fs/read_text_file', fs_write_text_file: 'fs/write_text_file', + authenticate_update: 'authenticate/update', session_request_permission: 'session/request_permission', session_update: 'session/update', }; @@ -57,8 +58,6 @@ export type CancelNotification = z.infer; export type AuthenticateRequest = z.infer; -export type AuthenticateResponse = z.infer; - export type NewSessionResponse = z.infer; export type LoadSessionResponse = z.infer; @@ -247,7 +246,13 @@ export const authenticateRequestSchema = z.object({ methodId: z.string(), }); -export const authenticateResponseSchema = z.null(); +export const authenticateUpdateSchema = z.object({ + _meta: z.object({ + authUri: z.string(), + }), +}); + +export type AuthenticateUpdate = z.infer; export const newSessionResponseSchema = z.object({ sessionId: z.string(), @@ -555,7 +560,6 @@ export const sessionUpdateSchema = z.union([ export const agentResponseSchema = z.union([ initializeResponseSchema, - authenticateResponseSchema, newSessionResponseSchema, loadSessionResponseSchema, promptResponseSchema, diff --git a/packages/core/src/qwen/qwenOAuth2.test.ts b/packages/core/src/qwen/qwenOAuth2.test.ts index 23c26296..0c401f90 100644 --- a/packages/core/src/qwen/qwenOAuth2.test.ts +++ b/packages/core/src/qwen/qwenOAuth2.test.ts @@ -761,7 +761,6 @@ describe('getQwenOAuthClient', () => { }); it('should load cached credentials if available', async () => { - const fs = await import('node:fs'); const mockCredentials = { access_token: 'cached-token', refresh_token: 'cached-refresh', @@ -769,10 +768,6 @@ describe('getQwenOAuthClient', () => { expiry_date: Date.now() + 3600000, }; - vi.mocked(fs.promises.readFile).mockResolvedValue( - JSON.stringify(mockCredentials), - ); - // Mock SharedTokenManager to use cached credentials const mockTokenManager = { getValidCredentials: vi.fn().mockResolvedValue(mockCredentials), @@ -792,18 +787,6 @@ describe('getQwenOAuthClient', () => { }); it('should handle cached credentials refresh failure', async () => { - const fs = await import('node:fs'); - const mockCredentials = { - access_token: 'cached-token', - refresh_token: 'expired-refresh', - token_type: 'Bearer', - expiry_date: Date.now() + 3600000, // Valid expiry time so loadCachedQwenCredentials returns true - }; - - vi.mocked(fs.promises.readFile).mockResolvedValue( - JSON.stringify(mockCredentials), - ); - // Mock SharedTokenManager to fail with a specific error const mockTokenManager = { getValidCredentials: vi @@ -833,6 +816,35 @@ describe('getQwenOAuthClient', () => { SharedTokenManager.getInstance = originalGetInstance; }); + + it('should not start device flow when requireCachedCredentials is true', async () => { + // Make SharedTokenManager fail so we hit the fallback path + const mockTokenManager = { + getValidCredentials: vi + .fn() + .mockRejectedValue(new Error('No credentials')), + }; + + const originalGetInstance = SharedTokenManager.getInstance; + SharedTokenManager.getInstance = vi.fn().mockReturnValue(mockTokenManager); + + // If requireCachedCredentials is honored, device-flow network requests should not start + vi.mocked(global.fetch).mockResolvedValue({ ok: true } as Response); + + await expect( + import('./qwenOAuth2.js').then((module) => + module.getQwenOAuthClient(mockConfig, { + requireCachedCredentials: true, + }), + ), + ).rejects.toThrow( + 'No cached Qwen-OAuth credentials found. Please re-authenticate.', + ); + + expect(global.fetch).not.toHaveBeenCalled(); + + SharedTokenManager.getInstance = originalGetInstance; + }); }); describe('CredentialsClearRequiredError', () => { @@ -1574,178 +1586,6 @@ describe('Credential Caching Functions', () => { expect(updatedCredentials.access_token).toBe('new-token'); }); }); - - describe('loadCachedQwenCredentials', () => { - it('should load and validate cached credentials successfully', async () => { - const { promises: fs } = await import('node:fs'); - const mockCredentials = { - access_token: 'cached-token', - refresh_token: 'cached-refresh', - token_type: 'Bearer', - expiry_date: Date.now() + 3600000, - }; - - vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(mockCredentials)); - - // Test through getQwenOAuthClient which calls loadCachedQwenCredentials - const mockConfig = { - isBrowserLaunchSuppressed: vi.fn().mockReturnValue(true), - } as unknown as Config; - - // Make SharedTokenManager fail to test the fallback - const mockTokenManager = { - getValidCredentials: vi - .fn() - .mockRejectedValue(new Error('No cached creds')), - }; - - const originalGetInstance = SharedTokenManager.getInstance; - SharedTokenManager.getInstance = vi - .fn() - .mockReturnValue(mockTokenManager); - - // Mock successful auth flow after cache load fails - const mockAuthResponse = { - ok: true, - json: async () => ({ - device_code: 'test-device-code', - user_code: 'TEST123', - verification_uri: 'https://chat.qwen.ai/device', - verification_uri_complete: 'https://chat.qwen.ai/device?code=TEST123', - expires_in: 1800, - }), - }; - - const mockTokenResponse = { - ok: true, - json: async () => ({ - access_token: 'new-access-token', - refresh_token: 'new-refresh-token', - token_type: 'Bearer', - expires_in: 3600, - scope: 'openid profile email model.completion', - }), - }; - - global.fetch = vi - .fn() - .mockResolvedValueOnce(mockAuthResponse as Response) - .mockResolvedValue(mockTokenResponse as Response); - - try { - await import('./qwenOAuth2.js').then((module) => - module.getQwenOAuthClient(mockConfig), - ); - } catch { - // Expected to fail in test environment - } - - expect(fs.readFile).toHaveBeenCalled(); - SharedTokenManager.getInstance = originalGetInstance; - }); - - it('should handle invalid cached credentials gracefully', async () => { - const { promises: fs } = await import('node:fs'); - - // Mock file read to return invalid JSON - vi.mocked(fs.readFile).mockResolvedValue('invalid-json'); - - const mockConfig = { - isBrowserLaunchSuppressed: vi.fn().mockReturnValue(true), - } as unknown as Config; - - const mockTokenManager = { - getValidCredentials: vi - .fn() - .mockRejectedValue(new Error('No cached creds')), - }; - - const originalGetInstance = SharedTokenManager.getInstance; - SharedTokenManager.getInstance = vi - .fn() - .mockReturnValue(mockTokenManager); - - // Mock auth flow - const mockAuthResponse = { - ok: true, - json: async () => ({ - device_code: 'test-device-code', - user_code: 'TEST123', - verification_uri: 'https://chat.qwen.ai/device', - verification_uri_complete: 'https://chat.qwen.ai/device?code=TEST123', - expires_in: 1800, - }), - }; - - const mockTokenResponse = { - ok: true, - json: async () => ({ - access_token: 'new-token', - refresh_token: 'new-refresh', - token_type: 'Bearer', - expires_in: 3600, - }), - }; - - global.fetch = vi - .fn() - .mockResolvedValueOnce(mockAuthResponse as Response) - .mockResolvedValue(mockTokenResponse as Response); - - try { - await import('./qwenOAuth2.js').then((module) => - module.getQwenOAuthClient(mockConfig), - ); - } catch { - // Expected to fail in test environment - } - - SharedTokenManager.getInstance = originalGetInstance; - }); - - it('should handle file access errors', async () => { - const { promises: fs } = await import('node:fs'); - - vi.mocked(fs.readFile).mockRejectedValue(new Error('File not found')); - - const mockConfig = { - isBrowserLaunchSuppressed: vi.fn().mockReturnValue(true), - } as unknown as Config; - - const mockTokenManager = { - getValidCredentials: vi - .fn() - .mockRejectedValue(new Error('No cached creds')), - }; - - const originalGetInstance = SharedTokenManager.getInstance; - SharedTokenManager.getInstance = vi - .fn() - .mockReturnValue(mockTokenManager); - - // Mock device flow to fail quickly - const mockAuthResponse = { - ok: true, - json: async () => ({ - error: 'invalid_request', - error_description: 'Invalid request parameters', - }), - }; - - global.fetch = vi.fn().mockResolvedValue(mockAuthResponse as Response); - - // Should proceed to device flow when cache loading fails - try { - await import('./qwenOAuth2.js').then((module) => - module.getQwenOAuthClient(mockConfig), - ); - } catch { - // Expected to fail in test environment - } - - SharedTokenManager.getInstance = originalGetInstance; - }); - }); }); describe('Enhanced Error Handling and Edge Cases', () => { diff --git a/packages/core/src/qwen/qwenOAuth2.ts b/packages/core/src/qwen/qwenOAuth2.ts index c4cfa933..3cb94a82 100644 --- a/packages/core/src/qwen/qwenOAuth2.ts +++ b/packages/core/src/qwen/qwenOAuth2.ts @@ -514,26 +514,14 @@ export async function getQwenOAuthClient( } } - // If shared manager fails, check if we have cached credentials for device flow - if (await loadCachedQwenCredentials(client)) { - // We have cached credentials but they might be expired - // Try device flow instead of forcing refresh - const result = await authWithQwenDeviceFlow(client, config); - if (!result.success) { - // Use detailed error message if available, otherwise use default - const errorMessage = - result.message || 'Qwen OAuth authentication failed'; - throw new Error(errorMessage); - } - return client; - } - if (options?.requireCachedCredentials) { throw new Error( 'No cached Qwen-OAuth credentials found. Please re-authenticate.', ); } + // If we couldn't obtain valid credentials via SharedTokenManager, fall back to + // interactive device authorization (unless explicitly forbidden above). const result = await authWithQwenDeviceFlow(client, config); if (!result.success) { // Only emit timeout event if the failure reason is actually timeout @@ -847,27 +835,6 @@ async function authWithQwenDeviceFlow( } } -async function loadCachedQwenCredentials( - client: QwenOAuth2Client, -): Promise { - try { - const keyFile = getQwenCachedCredentialPath(); - const creds = await fs.readFile(keyFile, 'utf-8'); - const credentials = JSON.parse(creds) as QwenCredentials; - client.setCredentials(credentials); - - // Verify that the credentials are still valid - const { token } = await client.getAccessToken(); - if (!token) { - return false; - } - - return true; - } catch (_) { - return false; - } -} - async function cacheQwenCredentials(credentials: QwenCredentials) { const filePath = getQwenCachedCredentialPath(); try {