-
Notifications
You must be signed in to change notification settings - Fork 747
fix(amazonq): Bug fixes for LSP auth on agentic mode #7231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -19,7 +19,6 @@ import api from './api' | |||
| import { activate as activateCWChat } from './app/chat/activation' | ||||
| import { activate as activateInlineChat } from './inlineChat/activation' | ||||
| import { beta } from 'aws-core-vscode/dev' | ||||
| import * as amazonq from 'aws-core-vscode/amazonq' | ||||
| import { activate as activateNotifications, NotificationsController } from 'aws-core-vscode/notifications' | ||||
| import { AuthUtil } from 'aws-core-vscode/codewhisperer' | ||||
| import { telemetry, AuthUserState } from 'aws-core-vscode/telemetry' | ||||
|
|
@@ -68,8 +67,6 @@ async function activateAmazonQNode(context: vscode.ExtensionContext) { | |||
| await activateTransformationHub(extContext as ExtContext) | ||||
| activateInlineChat(context) | ||||
|
|
||||
| context.subscriptions.push(amazonq.focusAmazonQPanel.register(), amazonq.focusAmazonQPanelKeybinding.register()) | ||||
|
|
||||
|
Comment on lines
-71
to
-72
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now part of
|
||||
| const authProvider = new CommonAuthViewProvider( | ||||
| context, | ||||
| amazonQContextPrefix, | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,19 +4,17 @@ | |
| */ | ||
|
|
||
| import vscode from 'vscode' | ||
| import { clientId, encryptionKey, startLanguageServer } from './client' | ||
| import { startLanguageServer } from './client' | ||
| import { AmazonQLspInstaller } from './lspInstaller' | ||
| import { lspSetupStage, ToolkitError, messages } from 'aws-core-vscode/shared' | ||
| import { AuthUtil } from 'aws-core-vscode/codewhisperer' | ||
| import { auth2 } from 'aws-core-vscode/auth' | ||
|
|
||
| export async function activate(ctx: vscode.ExtensionContext) { | ||
| try { | ||
| const client = await lspSetupStage('all', async () => { | ||
| await lspSetupStage('all', async () => { | ||
| const installResult = await new AmazonQLspInstaller().resolve() | ||
| return await lspSetupStage('launch', () => startLanguageServer(ctx, installResult.resourcePaths)) | ||
| }) | ||
| AuthUtil.create(new auth2.LanguageClientAuth(client, clientId, encryptionKey)) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move instantiation of the AuthUtil singleton into |
||
| await AuthUtil.instance.restore() | ||
| } catch (err) { | ||
| const e = err as ToolkitError | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,12 +23,14 @@ import { | |
| GetSsoTokenProgressToken, | ||
| GetSsoTokenProgressType, | ||
| MessageActionItem, | ||
| ShowDocumentParams, | ||
| ShowDocumentRequest, | ||
| ShowDocumentResult, | ||
| ShowMessageRequest, | ||
| ShowMessageRequestParams, | ||
| ConnectionMetadata, | ||
| ShowDocumentRequest, | ||
| ShowDocumentParams, | ||
| ShowDocumentResult, | ||
| ResponseError, | ||
| LSPErrorCodes, | ||
| } from '@aws/language-server-runtimes/protocol' | ||
| import { AuthUtil, CodeWhispererSettings, getSelectedCustomization } from 'aws-core-vscode/codewhisperer' | ||
| import { | ||
|
|
@@ -37,17 +39,17 @@ import { | |
| globals, | ||
| Experiments, | ||
| Commands, | ||
| openUrl, | ||
| validateNodeExe, | ||
| getLogger, | ||
| undefinedIfEmpty, | ||
| getOptOutPreference, | ||
| isAmazonInternalOs, | ||
| fs, | ||
| oidcClientName, | ||
| openUrl, | ||
| } from 'aws-core-vscode/shared' | ||
| import { processUtils } from 'aws-core-vscode/shared' | ||
| import { activate } from './chat/activation' | ||
| import { activate as activateChat } from './chat/activation' | ||
| import { AmazonQResourcePaths } from './lspInstaller' | ||
| import { auth2 } from 'aws-core-vscode/auth' | ||
| import { ConfigSection, isValidConfigSection, toAmazonQLSPLogLevel } from './config' | ||
|
|
@@ -144,9 +146,6 @@ export async function startLanguageServer( | |
| notifications: true, | ||
| showSaveFileDialog: true, | ||
| }, | ||
| q: { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this can be dropped
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The merge with mainline made it duplicate, see line 140 |
||
| developerProfiles: true, | ||
| }, | ||
| }, | ||
| logLevel: toAmazonQLSPLogLevel(globals.logOutputChannel.logLevel), | ||
| }, | ||
|
|
@@ -173,6 +172,7 @@ export async function startLanguageServer( | |
| toDispose.push(disposable) | ||
|
|
||
| await client.onReady() | ||
| AuthUtil.create(new auth2.LanguageClientAuth(client, clientId, encryptionKey)) | ||
|
|
||
| // Request handler for when the server wants to know about the clients auth connnection. Must be registered before the initial auth init call | ||
| client.onRequest<ConnectionMetadata, Error>(auth2.notificationTypes.getConnectionMetadata.method, () => { | ||
|
|
@@ -183,15 +183,6 @@ export async function startLanguageServer( | |
| } | ||
| }) | ||
|
|
||
| client.onRequest<ShowDocumentResult, Error>(ShowDocumentRequest.method, async (params: ShowDocumentParams) => { | ||
opieter-aws marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| try { | ||
| return { success: await openUrl(vscode.Uri.parse(params.uri), lspName) } | ||
| } catch (err: any) { | ||
| getLogger().error(`Failed to open document for LSP: ${lspName}, error: %s`, err) | ||
| return { success: false } | ||
| } | ||
| }) | ||
|
|
||
| client.onRequest<MessageActionItem | null, Error>( | ||
| ShowMessageRequest.method, | ||
| async (params: ShowMessageRequestParams) => { | ||
|
|
@@ -201,6 +192,30 @@ export async function startLanguageServer( | |
| } | ||
| ) | ||
|
|
||
| client.onRequest<ShowDocumentParams, ShowDocumentResult>( | ||
| ShowDocumentRequest.method, | ||
| async (params: ShowDocumentParams): Promise<ShowDocumentParams | ResponseError<ShowDocumentResult>> => { | ||
| const uri = vscode.Uri.parse(params.uri) | ||
| getLogger().info(`Processing ShowDocumentRequest for URI scheme: ${uri.scheme}`) | ||
| try { | ||
| if (uri.scheme.startsWith('http')) { | ||
| getLogger().info('Opening URL...') | ||
| await openUrl(vscode.Uri.parse(params.uri)) | ||
| } else { | ||
| getLogger().info('Opening text document...') | ||
| const doc = await vscode.workspace.openTextDocument(uri) | ||
| await vscode.window.showTextDocument(doc, { preview: false }) | ||
| } | ||
| return params | ||
| } catch (e) { | ||
| return new ResponseError( | ||
| LSPErrorCodes.RequestFailed, | ||
| `Failed to process ShowDocumentRequest: ${(e as Error).message}` | ||
| ) | ||
| } | ||
| } | ||
| ) | ||
|
|
||
| const sendProfileToLsp = async () => { | ||
| try { | ||
| const result = await client.sendRequest(updateConfigurationRequestType.method, { | ||
|
|
@@ -266,9 +281,7 @@ export async function startLanguageServer( | |
| ) | ||
| } | ||
|
|
||
| if (Experiments.instance.get('amazonqChatLSP', false)) { | ||
| activate(client, encryptionKey, resourcePaths.ui) | ||
| } | ||
| await activateChat(client, encryptionKey, resourcePaths.ui) | ||
|
|
||
| toDispose.push( | ||
| AuthUtil.instance.regionProfileManager.onDidChangeRegionProfile(sendProfileToLsp), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -88,6 +88,9 @@ export class AuthUtil implements IAuthProvider { | |
|
|
||
| async restore() { | ||
| await this.session.restore() | ||
| if (!this.isConnected()) { | ||
| await this.refreshState() | ||
| } | ||
|
Comment on lines
+91
to
+93
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This ensures that a session is restored successfully upon start of the extension with expired SSO. If SSO is expired upon start of extension, the LSP throws an error. Since there is no state change in that case ( |
||
| } | ||
|
|
||
| async login(startUrl: string, region: string) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This setup doesn't work with auth on LSP, since the LSP needs to start for auth to work