Merge master into feature/cwltail

Author: aws-toolkit-automation

packages/core/src/auth/auth.ts

@@ -126,8 +126,6 @@ export interface ConnectionStateChangeEvent {
     readonly state: ProfileMetadata['connectionState']
 }
 
-export type AuthType = Auth
-
 export type DeletedConnection = { connId: Connection['id']; storedProfile?: StoredProfile }
 type DeclaredConnection = Pick<SsoProfile, 'ssoRegion' | 'startUrl'> & { source: string }
 

packages/core/src/auth/connection.ts

@@ -24,6 +24,7 @@ const warnOnce = onceChanged((s: string, url: string) => {
     void showMessageWithUrl(s, url, undefined, 'warn')
 })
 
+// TODO: Refactor all scopes to a central file with minimal dependencies.
 export const scopesCodeCatalyst = ['codecatalyst:read_write']
 export const scopesSsoAccountAccess = ['sso:account:access']
 /** These are the non-chat scopes for CW. */
@@ -39,6 +40,11 @@ type SsoType =
     | 'idc' // AWS Identity Center
     | 'builderId'
 
+// TODO: This type is not centralized and there are many routines in the codebase that use some
+// variation for these for validation, telemetry, UX, etc. A refactor is needed to align these
+// string types.
+export type AuthType = 'credentials' | 'builderId' | 'identityCenter' | 'unknown'
+
 export const isIamConnection = (conn?: Connection): conn is IamConnection => conn?.type === 'iam'
 export const isSsoConnection = (conn?: Connection, type: SsoType = 'any'): conn is SsoConnection => {
     if (conn?.type !== 'sso') {

packages/core/src/auth/sso/validation.ts

@@ -4,7 +4,7 @@
  */
 import * as vscode from 'vscode'
 import { UnknownError } from '../../shared/errors'
-import { AuthType } from '../auth'
+import { Auth } from '../auth'
 import { SsoConnection, hasScopes, isAnySsoConnection } from '../connection'
 import { ssoUrlFormatMessage, ssoUrlFormatRegex } from './constants'
 
@@ -19,7 +19,7 @@ export function validateSsoUrlFormat(url: string) {
 }
 
 export async function validateIsNewSsoUrlAsync(
-    auth: AuthType,
+    auth: Auth,
     url: string,
     requiredScopes?: string[]
 ): Promise<string | undefined> {

packages/core/src/codewhisperer/util/supplementalContext/rankBm25.ts

@@ -7,7 +7,7 @@
 
 export interface BM25Document {
     content: string
-    /** The score that the document recieves. */
+    /** The score that the document receives. */
     score: number
 
     index: number

packages/core/src/notifications/activation.ts

@@ -11,9 +11,10 @@ import { RuleEngine, getRuleContext } from './rules'
 import globals from '../shared/extensionGlobals'
 import { AuthState } from './types'
 import { getLogger } from '../shared/logger/logger'
+import { oneMinute } from '../shared/datetime'
 
 /** Time in MS to poll for emergency notifications */
-const emergencyPollTime = 1000 * 10 * 60
+const emergencyPollTime = oneMinute * 10
 
 /**
  * Activate the in-IDE notifications module and begin receiving notifications.

packages/core/src/notifications/controller.ts

@@ -9,6 +9,7 @@ import globals from '../shared/extensionGlobals'
 import { globalKey } from '../shared/globalState'
 import {
     getNotificationTelemetryId,
+    Notifications,
     NotificationsState,
     NotificationsStateConstructor,
     NotificationType,
@@ -56,12 +57,7 @@ export class NotificationsController {
         }
         NotificationsController.#instance = this
 
-        this.state = globals.globalState.tryGet<NotificationsState>(this.storageKey, NotificationsStateConstructor, {
-            startUp: {},
-            emergency: {},
-            dismissed: [],
-            newlyReceived: [],
-        })
+        this.state = this.getDefaultState()
     }
 
     public pollForStartUp(ruleEngine: RuleEngine) {
@@ -74,6 +70,9 @@ export class NotificationsController {
 
     private async poll(ruleEngine: RuleEngine, category: NotificationType) {
         try {
+            // Get latest state in case it was modified by other windows.
+            // It is a minimal read to avoid race conditions.
+            this.readState()
             await this.fetchNotifications(category)
         } catch (err: any) {
             getLogger('notifications').error(`Unable to fetch %s notifications: %s`, category, err)
@@ -139,7 +138,7 @@ export class NotificationsController {
             return
         }
         // Parse the notifications
-        const newPayload = JSON.parse(response.content)
+        const newPayload: Notifications = JSON.parse(response.content)
         const newNotifications = newPayload.notifications ?? []
 
         // Get the current notifications
@@ -188,6 +187,33 @@ export class NotificationsController {
         await globals.globalState.update(this.storageKey, this.state)
     }
 
+    /**
+     * Read relevant values from the latest global state to memory. Useful to bring changes from other windows.
+     *
+     * Currently only brings dismissed, so users with multiple vscode instances open do not have issues with
+     * dismissing notifications multiple times. Otherwise, each instance has an independent session for
+     * displaying the notifications (e.g. multiple windows can be blocked in critical emergencies).
+     *
+     * Note: This sort of pattern (reading back and forth from global state in async functions) is prone to
+     * race conditions, which is why we limit the read to the fairly inconsequential `dismissed` property.
+     */
+    private readState() {
+        const state = this.getDefaultState()
+        this.state.dismissed = state.dismissed
+    }
+
+    /**
+     * Returns stored notification state, or a default state object if it is invalid or undefined.
+     */
+    private getDefaultState() {
+        return globals.globalState.tryGet<NotificationsState>(this.storageKey, NotificationsStateConstructor, {
+            startUp: {},
+            emergency: {},
+            dismissed: [],
+            newlyReceived: [],
+        })
+    }
+
     static get instance() {
         if (this.#instance === undefined) {
             throw new ToolkitError('NotificationsController was accessed before it has been initialized.')

packages/core/src/notifications/panelNode.ts

@@ -159,7 +159,8 @@ export class NotificationsNode implements TreeNode {
      * instructions included in the notification. See {@link ToolkitNotification.uiRenderInstructions}.
      */
     public async openNotification(notification: ToolkitNotification) {
-        switch (notification.uiRenderInstructions.onClick.type) {
+        const onClickType = notification.uiRenderInstructions.onClick.type
+        switch (onClickType) {
             case 'modal':
                 // Render blocking modal
                 logger.verbose(`rendering modal for notificaiton: ${notification.id} ...`)
@@ -180,7 +181,7 @@ export class NotificationsNode implements TreeNode {
                 // Display read-only txt document
                 logger.verbose(`showing txt document for notification: ${notification.id} ...`)
                 await telemetry.toolkit_invokeAction.run(async () => {
-                    telemetry.record({ source: getNotificationTelemetryId(notification), action: 'openTxt' })
+                    telemetry.record({ source: getNotificationTelemetryId(notification), action: onClickType })
                     await readonlyDocument.show(
                         notification.uiRenderInstructions.content['en-US'].description,
                         `Notification: ${notification.id}`
@@ -230,7 +231,7 @@ export class NotificationsNode implements TreeNode {
                         // Different button options
                         if (selectedButton) {
                             switch (selectedButton.type) {
-                                case 'openTxt':
+                                case 'openTextDocument':
                                     await readonlyDocument.show(
                                         notification.uiRenderInstructions.content['en-US'].description,
                                         `Notification: ${notification.id}`
@@ -260,7 +261,7 @@ export class NotificationsNode implements TreeNode {
 
     public async onReceiveNotifications(notifications: ToolkitNotification[]) {
         for (const notification of notifications) {
-            void this.showInformationWindow(notification, notification.uiRenderInstructions.onRecieve, true)
+            void this.showInformationWindow(notification, notification.uiRenderInstructions.onReceive, true)
         }
     }
 

packages/core/src/notifications/rules.ts

@@ -10,6 +10,7 @@ import { ConditionalClause, RuleContext, DisplayIf, CriteriaCondition, ToolkitNo
 import { getComputeEnvType, getOperatingSystem } from '../shared/telemetry/util'
 import { AuthFormId } from '../login/webview/vue/types'
 import { getLogger } from '../shared/logger/logger'
+import { ToolkitError } from '../shared/errors'
 
 const logger = getLogger('notifications')
 /**
@@ -158,7 +159,8 @@ export class RuleEngine {
             case 'ActiveExtensions':
                 return isSuperSetOfExpected(this.context.activeExtensions)
             default:
-                throw new Error(`Unknown criteria type: ${criteria.type}`)
+                logger.error('Unknown criteria passed to RuleEngine: %O', criteria)
+                throw new ToolkitError(`Unknown criteria type: ${(criteria as any).type}`)
         }
     }
 }

packages/core/src/notifications/types.ts

@@ -7,16 +7,55 @@ import * as vscode from 'vscode'
 import { EnvType, OperatingSystem } from '../shared/telemetry/util'
 import { TypeConstructor } from '../shared/utilities/typeConstructors'
 import { AuthUserState, AuthStatus } from '../shared/telemetry/telemetry.gen'
+import { AuthType } from '../auth/connection'
 
 /** Types of information that we can use to determine whether to show a notification or not. */
-export type Criteria = 'OS' | 'ComputeEnv' | 'AuthType' | 'AuthRegion' | 'AuthState' | 'AuthScopes' | 'ActiveExtensions'
 
-/** Generic condition where the type determines how the values are evaluated. */
-export interface CriteriaCondition {
-    readonly type: Criteria
-    readonly values: string[]
+type OsCriteria = {
+    type: 'OS'
+    values: OperatingSystem[]
+}
+
+type ComputeEnvCriteria = {
+    type: 'ComputeEnv'
+    values: EnvType[]
+}
+
+type AuthTypeCriteria = {
+    type: 'AuthType'
+    values: AuthType[]
+}
+
+type AuthRegionCriteria = {
+    type: 'AuthRegion'
+    values: string[]
+}
+
+type AuthStateCriteria = {
+    type: 'AuthState'
+    values: AuthStatus[]
+}
+
+type AuthScopesCriteria = {
+    type: 'AuthScopes'
+    values: string[] // TODO: Scopes should be typed. Could import here, but don't want to import too much.
+}
+
+type ActiveExtensionsCriteria = {
+    type: 'ActiveExtensions'
+    values: string[]
 }
 
+/** Generic condition where the type determines how the values are evaluated. */
+export type CriteriaCondition =
+    | OsCriteria
+    | ComputeEnvCriteria
+    | AuthTypeCriteria
+    | AuthRegionCriteria
+    | AuthStateCriteria
+    | AuthScopesCriteria
+    | ActiveExtensionsCriteria
+
 /** One of the subconditions (clauses) must match to be valid. */
 export interface OR {
     readonly type: 'or'
@@ -39,8 +78,8 @@ export interface ExactMatch {
 export type ConditionalClause = Range | ExactMatch | OR
 
 export type OnReceiveType = 'toast' | 'modal'
-export type OnClickType = 'modal' | 'openTextDocument' | 'openUrl'
-export type ActionType = 'openUrl' | 'updateAndReload' | 'openTxt'
+export type OnClickType = { type: 'modal' } | { type: 'openTextDocument' } | { type: 'openUrl'; url: string }
+export type ActionType = 'openUrl' | 'updateAndReload' | 'openTextDocument'
 
 /** How to display the notification. */
 export interface UIRenderInstructions {
@@ -51,11 +90,8 @@ export interface UIRenderInstructions {
             toastPreview?: string // optional property for toast
         }
     }
-    onRecieve: OnReceiveType // TODO: typo
-    onClick: {
-        type: OnClickType
-        url?: string // optional property for 'openUrl'
-    }
+    onReceive: OnReceiveType
+    onClick: OnClickType
     actions?: Array<{
         type: ActionType
         displayText: {
@@ -124,7 +160,7 @@ export interface RuleContext {
     readonly extensionVersion: string
     readonly os: OperatingSystem
     readonly computeEnv: EnvType
-    readonly authTypes: ('credentials' | 'builderId' | 'identityCenter' | 'unknown')[]
+    readonly authTypes: AuthType[]
     readonly authRegions: string[]
     readonly authStates: AuthStatus[]
     readonly authScopes: string[]

packages/core/src/test/awsService/ec2/model.test.ts

@@ -24,7 +24,7 @@ describe('Ec2ConnectClient', function () {
     })
 
     describe('getAttachedIamRole', async function () {
-        it('only returns role if recieves ARN from instance profile', async function () {
+        it('only returns role if receives ARN from instance profile', async function () {
             let role: IAM.Role | undefined
             const getInstanceProfileStub = sinon.stub(Ec2Client.prototype, 'getAttachedIamInstanceProfile')