diff --git a/src/vs/platform/agentHost/common/agentService.ts b/src/vs/platform/agentHost/common/agentService.ts index 14deb197fa3..7cbe07bdd7e 100644 --- a/src/vs/platform/agentHost/common/agentService.ts +++ b/src/vs/platform/agentHost/common/agentService.ts @@ -388,8 +388,9 @@ export interface IAgentService { getResourceMetadata(): Promise; /** - * Authenticate with the server using a specific auth scheme. - * Analogous to sending `Authorization: Bearer ` (RFC 6750). + * Authenticate for a protected resource on the server. + * The {@link IAuthenticateParams.resource} must match a resource from + * {@link getResourceMetadata}. Analogous to RFC 6750 bearer token delivery. */ authenticate(params: IAuthenticateParams): Promise; diff --git a/src/vs/platform/agentHost/node/protocolServerHandler.ts b/src/vs/platform/agentHost/node/protocolServerHandler.ts index e655008bdd5..ef47a34c59f 100644 --- a/src/vs/platform/agentHost/node/protocolServerHandler.ts +++ b/src/vs/platform/agentHost/node/protocolServerHandler.ts @@ -41,6 +41,15 @@ function jsonRpcError(id: number, code: number, message: string, data?: unknown) return { jsonrpc: '2.0', id, error: { code, message, ...(data !== undefined ? { data } : {}) } }; } +/** Build a JSON-RPC error response from an unknown thrown value, preserving {@link ProtocolError} fields. */ +function jsonRpcErrorFrom(id: number, err: unknown): IJsonRpcResponse { + if (err instanceof ProtocolError) { + return jsonRpcError(id, err.code, err.message, err.data); + } + const message = err instanceof Error ? (err.stack ?? err.message) : String(err); + return jsonRpcError(id, JSON_RPC_INTERNAL_ERROR, message); +} + /** * Methods handled by the request dispatcher. Excludes `initialize` and * `reconnect` which are handled during the handshake phase. @@ -119,9 +128,7 @@ export class ProtocolServerHandler extends Disposable { client = result.client; transport.send(jsonRpcSuccess(msg.id, result.response)); } catch (err) { - const code = err instanceof ProtocolError ? err.code : JSON_RPC_INTERNAL_ERROR; - const message = err instanceof Error ? err.message : String(err); - transport.send(jsonRpcError(msg.id, code, message)); + transport.send(jsonRpcErrorFrom(msg.id, err)); } return; } @@ -131,9 +138,7 @@ export class ProtocolServerHandler extends Disposable { client = result.client; transport.send(jsonRpcSuccess(msg.id, result.response)); } catch (err) { - const code = err instanceof ProtocolError ? err.code : JSON_RPC_INTERNAL_ERROR; - const message = err instanceof Error ? err.message : String(err); - transport.send(jsonRpcError(msg.id, code, message)); + transport.send(jsonRpcErrorFrom(msg.id, err)); } return; } @@ -333,14 +338,7 @@ export class ProtocolServerHandler extends Disposable { client.transport.send(jsonRpcSuccess(id, result ?? null)); }).catch(err => { this._logService.error(`[ProtocolServer] Request '${method}' failed`, err); - const code = err instanceof ProtocolError ? err.code : JSON_RPC_INTERNAL_ERROR; - const data = err instanceof ProtocolError ? err.data : undefined; - const message = err instanceof ProtocolError - ? err.message - : err instanceof Error && err.stack - ? err.stack - : String(err?.message ?? err); - client.transport.send(jsonRpcError(id, code, message, data)); + client.transport.send(jsonRpcErrorFrom(id, err)); }); return; } @@ -352,7 +350,7 @@ export class ProtocolServerHandler extends Disposable { client.transport.send(jsonRpcSuccess(id, result ?? null)); }).catch(err => { this._logService.error(`[ProtocolServer] Extension request '${method}' failed`, err); - client.transport.send(jsonRpcError(id, JSON_RPC_INTERNAL_ERROR, String(err?.message ?? err))); + client.transport.send(jsonRpcErrorFrom(id, err)); }); return; } diff --git a/src/vs/platform/agentHost/test/auth-rework.md b/src/vs/platform/agentHost/test/auth-rework.md index eb1f67709bc..4533c3b4e59 100644 --- a/src/vs/platform/agentHost/test/auth-rework.md +++ b/src/vs/platform/agentHost/test/auth-rework.md @@ -240,7 +240,7 @@ This is returned as the `data` payload of a JSON-RPC error response: "jsonrpc": "2.0", "id": 5, "error": { - "code": -32010, + "code": -32007, "message": "Authentication required", "data": { "challenges": [ @@ -255,7 +255,7 @@ This is returned as the `data` payload of a JSON-RPC error response: } ``` -A dedicated error code (e.g. `-32010 AHP_AUTH_REQUIRED`) signals this is an auth error so clients can handle it programmatically without parsing the message string. +A dedicated error code (`-32007 AHP_AUTH_REQUIRED`) signals this is an auth error so clients can handle it programmatically without parsing the message string. ### Phase 4: Auth State Notifications @@ -356,7 +356,7 @@ for (const scheme of initResult.resourceMetadata?.authSchemes ?? []) { | Code | Name | When | |---|---|---| -| `-32010` | `AHP_AUTH_REQUIRED` | A command failed because auth is missing or invalid | +| `-32007` | `AHP_AUTH_REQUIRED` | A command failed because auth is missing or invalid | ### Extended: `initialize` result diff --git a/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts b/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts index 23be5c8c3fa..3eac4562547 100644 --- a/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts +++ b/src/vs/platform/agentHost/test/node/protocolServerHandler.test.ts @@ -381,4 +381,50 @@ suite('ProtocolServerHandler', () => { assert.strictEqual(resp.error!.code, JSON_RPC_INTERNAL_ERROR); assert.match(resp.error!.message, /Directory not found/); }); + + // ---- Extension methods: auth ---------------------------------------- + + test('getResourceMetadata returns resource metadata via extension request', async () => { + const transport = connectClient('client-metadata'); + transport.sent.length = 0; + + const responsePromise = waitForResponse(transport, 2); + transport.simulateMessage(request(2, 'getResourceMetadata')); + const resp = await responsePromise as { result?: { resources: unknown[] } }; + + assert.ok(resp?.result); + assert.ok(Array.isArray(resp.result!.resources)); + }); + + test('authenticate returns result via extension request', async () => { + const transport = connectClient('client-auth'); + transport.sent.length = 0; + + const responsePromise = waitForResponse(transport, 2); + transport.simulateMessage(request(2, 'authenticate', { resource: 'https://api.github.com', token: 'test-token' })); + const resp = await responsePromise as { result?: { authenticated: boolean } }; + + assert.ok(resp?.result); + assert.strictEqual(resp.result!.authenticated, true); + }); + + test('extension request preserves ProtocolError code and data', async () => { + // Override handleAuthenticate to throw a ProtocolError with data + const origHandler = sideEffects.handleAuthenticate; + sideEffects.handleAuthenticate = async () => { throw new ProtocolError(-32007, 'Auth required', { hint: 'sign in' }); }; + + const transport = connectClient('client-auth-error'); + transport.sent.length = 0; + + const responsePromise = waitForResponse(transport, 2); + transport.simulateMessage(request(2, 'authenticate', { resource: 'test', token: 'bad' })); + const resp = await responsePromise as { error?: { code: number; message: string; data?: unknown } }; + + assert.ok(resp?.error); + assert.strictEqual(resp.error!.code, -32007); + assert.strictEqual(resp.error!.message, 'Auth required'); + assert.deepStrictEqual(resp.error!.data, { hint: 'sign in' }); + + sideEffects.handleAuthenticate = origHandler; + }); }); diff --git a/src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHost.contribution.ts b/src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHost.contribution.ts index 870428fda7e..92941d06af8 100644 --- a/src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHost.contribution.ts +++ b/src/vs/sessions/contrib/remoteAgentHost/browser/remoteAgentHost.contribution.ts @@ -103,8 +103,8 @@ export class RemoteAgentHostContribution extends Disposable implements IWorkbenc })); // Push auth token whenever the default account or sessions change - this._register(this._defaultAccountService.onDidChangeDefaultAccount(() => this._pushAuthTokenToAll())); - this._register(this._authenticationService.onDidChangeSessions(() => this._pushAuthTokenToAll())); + this._register(this._defaultAccountService.onDidChangeDefaultAccount(() => this._authenticateAllConnections())); + this._register(this._authenticationService.onDidChangeSessions(() => this._authenticateAllConnections())); // Initial setup for already-connected remotes this._reconcileConnections(); @@ -300,7 +300,7 @@ export class RemoteAgentHostContribution extends Disposable implements IWorkbenc this._logService.info(`[RemoteAgentHost] Registered agent ${agent.provider} from ${address} as ${sessionType}`); } - private _pushAuthTokenToAll(): void { + private _authenticateAllConnections(): void { for (const address of this._connections.keys()) { const connection = this._remoteAgentHostService.getConnection(address); if (connection) { @@ -318,7 +318,8 @@ export class RemoteAgentHostContribution extends Disposable implements IWorkbenc try { const metadata = await connection.getResourceMetadata(); for (const resource of metadata.resources) { - const token = await this._resolveTokenForResource(resource.authorization_servers ?? [], resource.scopes_supported ?? []); + const resourceUri = URI.parse(resource.resource); + const token = await this._resolveTokenForResource(resourceUri, resource.authorization_servers ?? [], resource.scopes_supported ?? []); if (token) { this._logService.info(`[RemoteAgentHost] Authenticating for resource: ${resource.resource}`); await connection.authenticate({ resource: resource.resource, token }); @@ -335,10 +336,10 @@ export class RemoteAgentHostContribution extends Disposable implements IWorkbenc * Resolve a bearer token for a set of authorization servers using the * standard VS Code authentication service provider resolution. */ - private async _resolveTokenForResource(authorizationServers: readonly string[], scopes: readonly string[]): Promise { + private async _resolveTokenForResource(resourceServer: URI, authorizationServers: readonly string[], scopes: readonly string[]): Promise { for (const server of authorizationServers) { const serverUri = URI.parse(server); - const providerId = await this._authenticationService.getOrActivateProviderIdForServer(serverUri); + const providerId = await this._authenticationService.getOrActivateProviderIdForServer(serverUri, resourceServer); if (!providerId) { this._logService.trace(`[RemoteAgentHost] No auth provider found for server: ${server}`); continue; @@ -348,12 +349,6 @@ export class RemoteAgentHostContribution extends Disposable implements IWorkbenc if (sessions.length > 0) { return sessions[0].accessToken; } - - // Fall back to any session from the provider - const anySessions = await this._authenticationService.getSessions(providerId); - if (anySessions.length > 0) { - return anySessions[0].accessToken; - } } return undefined; } @@ -368,7 +363,8 @@ export class RemoteAgentHostContribution extends Disposable implements IWorkbenc for (const resource of metadata.resources) { for (const server of resource.authorization_servers ?? []) { const serverUri = URI.parse(server); - const providerId = await this._authenticationService.getOrActivateProviderIdForServer(serverUri); + const resourceUri = URI.parse(resource.resource); + const providerId = await this._authenticationService.getOrActivateProviderIdForServer(serverUri, resourceUri); if (!providerId) { continue; } diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts index 92495b2b93a..c87e45bead2 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostChatContribution.ts @@ -251,7 +251,8 @@ export class AgentHostContribution extends Disposable implements IWorkbenchContr const metadata = await this._agentHostService.getResourceMetadata(); this._logService.trace(`[AgentHost] Resource metadata: ${metadata.resources.length} resource(s)`); for (const resource of metadata.resources) { - const token = await this._resolveTokenForResource(resource.authorization_servers ?? [], resource.scopes_supported ?? []); + const resourceUri = URI.parse(resource.resource); + const token = await this._resolveTokenForResource(resourceUri, resource.authorization_servers ?? [], resource.scopes_supported ?? []); if (token) { this._logService.info(`[AgentHost] Authenticating for resource: ${resource.resource}`); await this._agentHostService.authenticate({ resource: resource.resource, token }); @@ -268,30 +269,21 @@ export class AgentHostContribution extends Disposable implements IWorkbenchContr * Resolve a bearer token for a set of authorization servers using the * standard VS Code authentication service provider resolution. */ - private async _resolveTokenForResource(authorizationServers: readonly string[], scopes: readonly string[]): Promise { + private async _resolveTokenForResource(resourceServer: URI, authorizationServers: readonly string[], scopes: readonly string[]): Promise { for (const server of authorizationServers) { const serverUri = URI.parse(server); - const providerId = await this._authenticationService.getOrActivateProviderIdForServer(serverUri); + const providerId = await this._authenticationService.getOrActivateProviderIdForServer(serverUri, resourceServer); if (!providerId) { this._logService.trace(`[AgentHost] No auth provider found for server: ${server}`); continue; } this._logService.trace(`[AgentHost] Resolved auth provider '${providerId}' for server: ${server}`); - // Try with the declared scopes first, then fall back to empty scopes - // (the provider may have sessions with broader scopes). const sessions = await this._authenticationService.getSessions(providerId, [...scopes], { authorizationServer: serverUri }, true); if (sessions.length > 0) { return sessions[0].accessToken; } - // Fall back to any session from the provider - const anySessions = await this._authenticationService.getSessions(providerId); - if (anySessions.length > 0) { - this._logService.trace(`[AgentHost] Using session with broader scopes from provider '${providerId}'`); - return anySessions[0].accessToken; - } - this._logService.trace(`[AgentHost] No sessions found for provider '${providerId}'`); } return undefined; @@ -309,7 +301,8 @@ export class AgentHostContribution extends Disposable implements IWorkbenchContr for (const resource of metadata.resources) { for (const server of resource.authorization_servers ?? []) { const serverUri = URI.parse(server); - const providerId = await this._authenticationService.getOrActivateProviderIdForServer(serverUri); + const resourceUri = URI.parse(resource.resource); + const providerId = await this._authenticationService.getOrActivateProviderIdForServer(serverUri, resourceUri); if (!providerId) { continue; } diff --git a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts index a3f7c47ec5f..c12d76c2766 100644 --- a/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts +++ b/src/vs/workbench/contrib/chat/browser/agentSessions/agentHost/agentHostSessionHandler.ts @@ -7,6 +7,7 @@ import { CancellationToken } from '../../../../../../base/common/cancellation.js import { Emitter } from '../../../../../../base/common/event.js'; import { MarkdownString } from '../../../../../../base/common/htmlContent.js'; import { Disposable, DisposableStore, toDisposable } from '../../../../../../base/common/lifecycle.js'; +import { localize } from '../../../../../../nls.js'; import { observableValue } from '../../../../../../base/common/observable.js'; import { generateUuid } from '../../../../../../base/common/uuid.js'; import { URI } from '../../../../../../base/common/uri.js'; @@ -17,6 +18,7 @@ import { IProductService } from '../../../../../../platform/product/common/produ import { IWorkspaceContextService } from '../../../../../../platform/workspace/common/workspace.js'; import { IAgentAttachment, AgentProvider, AgentSession, type IAgentConnection } from '../../../../../../platform/agentHost/common/agentService.js'; import { ActionType, isSessionAction } from '../../../../../../platform/agentHost/common/state/sessionActions.js'; +import { AHP_AUTH_REQUIRED, ProtocolError } from '../../../../../../platform/agentHost/common/state/sessionProtocol.js'; import { SessionClientState } from '../../../../../../platform/agentHost/common/state/sessionClientState.js'; import { getToolKind, getToolLanguage } from '../../../../../../platform/agentHost/common/state/sessionReducers.js'; import { AttachmentType, ToolCallStatus, TurnState, type IMessageAttachment } from '../../../../../../platform/agentHost/common/state/sessionState.js'; @@ -468,7 +470,7 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC workingDirectory, }); } else { - throw new Error('Authentication is required to start a session. Please sign in and try again.'); + throw new Error(localize('agentHost.authRequired', "Authentication is required to start a session. Please sign in and try again.")); } } else { throw err; @@ -490,9 +492,14 @@ export class AgentHostSessionHandler extends Disposable implements IChatSessionC /** * Check if an error is an "authentication required" error. - * Works across both ProxyChannel (message-only) and WebSocket (structured) paths. + * Checks for the AHP_AUTH_REQUIRED error code when available, + * with a message-based fallback for transports that don't preserve + * structured error codes (e.g. ProxyChannel). */ private _isAuthRequiredError(err: unknown): boolean { + if (err instanceof ProtocolError && err.code === AHP_AUTH_REQUIRED) { + return true; + } if (err instanceof Error && err.message.includes('Authentication required')) { return true; }