fix(notifications): update types, field names, cross-window syncing (#…

…6040)

- Make types more granular.
- Align names - fix onRecieve typo, openTxt -> openTextDocument
- Sync from global state on reload to detect activity in other windows.
This makes it so that if users dismiss in one window, it will dismiss in
all windows. It will also prevent a bug where users may get dismissed
notifications again if they have multiple windows.

---

<!--- REMINDER: Ensure that your PR meets the guidelines in
CONTRIBUTING.md -->

License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.

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')
 

packages/core/src/test/notifications/controller.test.ts

@@ -557,7 +557,7 @@ function getValidTestNotification(id: string): ToolkitNotification {
                     description: 'test',
                 },
             },
-            onRecieve: 'toast',
+            onReceive: 'toast',
             onClick: {
                 type: 'openUrl',
                 url: 'https://aws.amazon.com/visualstudiocode/',
@@ -580,7 +580,7 @@ function getInvalidTestNotification(id: string): ToolkitNotification {
                     description: 'test',
                 },
             },
-            onRecieve: 'toast',
+            onReceive: 'toast',
             onClick: { type: 'modal' },
         },
     }

packages/core/src/test/notifications/rendering.test.ts

@@ -112,7 +112,7 @@ describe('Notifications Rendering', function () {
         assert.ok(telemetrySpy.calledOnce)
     })
 
-    it('executes openURL type button', async function () {
+    it('executes openUrl type button', async function () {
         const testWindow = getTestWindow()
         testWindow.onDidShowMessage((message) => {
             // Simulate user clicking open URL type
@@ -122,7 +122,7 @@ describe('Notifications Rendering', function () {
         await verifyOpenExternalUrl(notification)
     })
 
-    it('executes openTxt type button', async function () {
+    it('executes openTextDocument type button', async function () {
         const testWindow = getTestWindow()
         testWindow.onDidShowMessage((message) => {
             // Simulate user clicking open txt type
@@ -148,7 +148,7 @@ function getToastURLTestNotification(): ToolkitNotification {
                     toastPreview: 'test toast preview',
                 },
             },
-            onRecieve: 'toast',
+            onReceive: 'toast',
             onClick: {
                 type: 'openUrl',
                 url: 'https://aws.amazon.com/visualstudiocode/',
@@ -170,7 +170,7 @@ function getTxtNotification(): ToolkitNotification {
                     description: 'This is a text document notification.',
                 },
             },
-            onRecieve: 'toast',
+            onReceive: 'toast',
             onClick: {
                 type: 'openTextDocument',
             },
@@ -191,7 +191,7 @@ function getModalNotification(): ToolkitNotification {
                     description: 'This is a modal notification.',
                 },
             },
-            onRecieve: 'modal',
+            onReceive: 'modal',
             onClick: {
                 type: 'modal',
             },
@@ -210,7 +210,7 @@ function getModalNotification(): ToolkitNotification {
                     },
                 },
                 {
-                    type: 'openTxt',
+                    type: 'openTextDocument',
                     displayText: {
                         'en-US': 'Read More',
                     },

packages/core/src/test/notifications/resources/emergency/1.x.json

@@ -14,7 +14,7 @@
                         "toastPreview": "Signing into Amazon Q is broken, please try this workaround while we work on releasing a fix."
                     }
                 },
-                "onRecieve": "toast",
+                "onReceive": "toast",
                 "onClick": {
                     "type": "openTextDocument"
                 },
@@ -34,7 +34,7 @@
                         "toastPreview": "This version of Amazon Q is currently broken, please update to avoid issues."
                     }
                 },
-                "onRecieve": "toast",
+                "onReceive": "toast",
                 "onClick": {
                     "type": "modal"
                 },