fix(amazonq): Sync client settings to language server #7114
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
zixlin7
reviewed
Apr 20, 2025
| return [ | ||
| { | ||
| customization, | ||
| optOutTelemetry: getOptOutPreference() === 'OPTOUT', |
Contributor
There was a problem hiding this comment.
oh wait this is a boolean? the type they have in the interface vs what is expected to be passed in is so confusing 😅
Contributor
Author
There was a problem hiding this comment.
Yeah its weird lol. They expect a boolean then convert that back in to an 'OPTIN' | 'OPTOUT' string:
https://github.com/aws/language-servers/blob/c367ef3a1374032dace0e6755eaa43a1fae6e3c4/server/aws-lsp-codewhisperer/src/shared/amazonQServiceManager/configurationUtils.ts#L117
Just needed to fix the value to match that of the one in configurationUtils.ts Signed-off-by: nkomonen-amazon <nkomonen@amazon.com>
Problem: A message was already set up in a previous commit to send the lsp a message when the Q customization model was changed, but it didn't work as expected. Solution: There are different lsp messages sent to indicate a change in user settings. For auth Q profile it is one type of Request, and for the customization selection, it uses a different lsp notification. Each type of message has its own handler in the language server, so if we send the message using the wrong message type, it will not be handled correctly since the correct handler is somewhere else. The notable handling functions were: - getAmazonQRelatedWorkspaceConfigs(), where it is part of the handler for `didChangeConfiguration()` - setupConfigurationListeners(), where it is part of the handler for `onUpdateConfiguration()` Signed-off-by: nkomonen-amazon <nkomonen@amazon.com>
- Remove some unrelated development related code - Enable local indexing Signed-off-by: nkomonen-amazon <nkomonen@amazon.com>
nkomonen-amazon
force-pushed
the
flare-local-context-2
branch
from
fdb6315 to
f303e56
Compare
jpinkney-aws
approved these changes
Apr 20, 2025
justinmk3
reviewed
Apr 23, 2025
| await Commands.tryExecute('aws.amazonq.refreshStatusBar') | ||
|
|
||
| // hack: triggers amazon q to send the customizations back to flare | ||
| await Commands.tryExecute('aws.amazonq.updateCustomizations') |
Contributor
There was a problem hiding this comment.
Why not call a function instead of a command?
Commands should be avoided, because they are less reliable and less visible if they fail (partly because they tend to be async).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When certain settings/configs are changed in the IDE, they need to be propagated to the language server.
The settings were the customizationArn, the users opt in preference for telemetry, and some other things.
This PR:
aws.amazonq.updateCustomizationswhich was made to handle this edge case. Which eventually explicitly uses the language server messageDidChangeConfigurationNotificationDidChangeConfigurationNotificationmessage.pushConfigUpdate()was created to route the correct config to the correct handler in the LSupdateConfigurationRequestTypewhich is different fromDidChangeConfigurationNotificationthat the other configs use. This results in a different code path being hit, which is expected.Some helpful notes:
getAmazonQRelatedWorkspaceConfigs()(triggered throughDidChangeConfigurationNotification) is the language server side code that parses the config sent by the client/ide. So the client/ide must match the shape expected by the LS. This is an example whyoptOutTelemetrydid not work out of the box, and needed some changes to be consumable by the LS.feature/xbranches will not be squash-merged at release time.