diff --git a/packages/core/src/auth/auth.ts b/packages/core/src/auth/auth.ts index 334c3841216..3335f8ab333 100644 --- a/packages/core/src/auth/auth.ts +++ b/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 & { source: string } diff --git a/packages/core/src/auth/connection.ts b/packages/core/src/auth/connection.ts index 5fc505a0b5f..3e7752dd8e9 100644 --- a/packages/core/src/auth/connection.ts +++ b/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') { diff --git a/packages/core/src/auth/sso/validation.ts b/packages/core/src/auth/sso/validation.ts index ecfe935a837..cad6aa7f4c3 100644 --- a/packages/core/src/auth/sso/validation.ts +++ b/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 { diff --git a/packages/core/src/codewhisperer/util/supplementalContext/rankBm25.ts b/packages/core/src/codewhisperer/util/supplementalContext/rankBm25.ts index a6fd84aafd6..3e00013c9cf 100644 --- a/packages/core/src/codewhisperer/util/supplementalContext/rankBm25.ts +++ b/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 diff --git a/packages/core/src/notifications/activation.ts b/packages/core/src/notifications/activation.ts index 2ac36bcd5bd..cd732909d16 100644 --- a/packages/core/src/notifications/activation.ts +++ b/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. diff --git a/packages/core/src/notifications/controller.ts b/packages/core/src/notifications/controller.ts index a93835d89b1..30aa8ef3570 100644 --- a/packages/core/src/notifications/controller.ts +++ b/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(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(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.') diff --git a/packages/core/src/notifications/panelNode.ts b/packages/core/src/notifications/panelNode.ts index 94e4f2031f1..fe2512be47b 100644 --- a/packages/core/src/notifications/panelNode.ts +++ b/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) } } diff --git a/packages/core/src/notifications/rules.ts b/packages/core/src/notifications/rules.ts index 9e62adb7cc1..34f5660a50e 100644 --- a/packages/core/src/notifications/rules.ts +++ b/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}`) } } } diff --git a/packages/core/src/notifications/types.ts b/packages/core/src/notifications/types.ts index 59717cf9079..5ee06df5cd9 100644 --- a/packages/core/src/notifications/types.ts +++ b/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[] diff --git a/packages/core/src/test/awsService/ec2/model.test.ts b/packages/core/src/test/awsService/ec2/model.test.ts index 6c7f6f1fb4a..d39de4ba1da 100644 --- a/packages/core/src/test/awsService/ec2/model.test.ts +++ b/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') diff --git a/packages/core/src/test/notifications/controller.test.ts b/packages/core/src/test/notifications/controller.test.ts index c1e2d299463..25b013db71b 100644 --- a/packages/core/src/test/notifications/controller.test.ts +++ b/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' }, }, } diff --git a/packages/core/src/test/notifications/rendering.test.ts b/packages/core/src/test/notifications/rendering.test.ts index e446e53fdba..ccda76195a2 100644 --- a/packages/core/src/test/notifications/rendering.test.ts +++ b/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', }, diff --git a/packages/core/src/test/notifications/resources/emergency/1.x.json b/packages/core/src/test/notifications/resources/emergency/1.x.json index ced10965ab5..b0a83f40d3b 100644 --- a/packages/core/src/test/notifications/resources/emergency/1.x.json +++ b/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" }, diff --git a/packages/core/src/test/notifications/resources/startup/1.x.json b/packages/core/src/test/notifications/resources/startup/1.x.json index 29a7e7e89cd..aae5b1d70f2 100644 --- a/packages/core/src/test/notifications/resources/startup/1.x.json +++ b/packages/core/src/test/notifications/resources/startup/1.x.json @@ -14,7 +14,7 @@ "toastPreview": "New Amazon Q features available: inline chat" } }, - "onRecieve": "toast", + "onReceive": "toast", "onClick": { "type": "openTextDocument" }, @@ -41,7 +41,7 @@ "toastPreview": "New Amazon Q features are available!" } }, - "onRecieve": "toast", + "onReceive": "toast", "onClick": { "type": "openUrl", "url": "https://aws.amazon.com/developer/generative-ai/amazon-q/change-log/" diff --git a/packages/core/src/test/notifications/rules.test.ts b/packages/core/src/test/notifications/rules.test.ts index 867b2119546..0df695c1be6 100644 --- a/packages/core/src/test/notifications/rules.test.ts +++ b/packages/core/src/test/notifications/rules.test.ts @@ -40,7 +40,7 @@ describe('Notifications Rule Engine', function () { description: 'Something crazy is happening! Please update your extension.', }, }, - onRecieve: 'toast', + onReceive: 'toast', onClick: { type: 'openUrl', url: 'https://aws.amazon.com/visualstudiocode/', @@ -305,7 +305,7 @@ describe('Notifications Rule Engine', function () { assert.equal( ruleEngine.shouldDisplayNotification( buildNotification({ - additionalCriteria: [{ type: 'AuthType', values: ['builderId', 'iamIdentityCenter'] }], + additionalCriteria: [{ type: 'AuthType', values: ['builderId', 'identityCenter'] }], }) ), true @@ -316,7 +316,7 @@ describe('Notifications Rule Engine', function () { assert.equal( ruleEngine.shouldDisplayNotification( buildNotification({ - additionalCriteria: [{ type: 'AuthType', values: ['iamIdentityCenter'] }], + additionalCriteria: [{ type: 'AuthType', values: ['identityCenter'] }], }) ), false @@ -452,7 +452,7 @@ describe('Notifications Rule Engine', function () { additionalCriteria: [ { type: 'OS', values: ['LINUX', 'MAC'] }, { type: 'ComputeEnv', values: ['local', 'ec2'] }, - { type: 'AuthType', values: ['builderId', 'iamIdentityCenter'] }, + { type: 'AuthType', values: ['builderId', 'identityCenter'] }, { type: 'AuthRegion', values: ['us-east-1', 'us-west-2'] }, { type: 'AuthState', values: ['connected'] }, { type: 'AuthScopes', values: ['codewhisperer:completions', 'codewhisperer:analysis'] }, @@ -489,7 +489,7 @@ describe('Notifications Rule Engine', function () { }, additionalCriteria: [ { type: 'OS', values: ['LINUX', 'MAC'] }, - { type: 'AuthType', values: ['builderId', 'iamIdentityCenter'] }, + { type: 'AuthType', values: ['builderId', 'identityCenter'] }, { type: 'AuthRegion', values: ['us-east-1', 'us-west-2'] }, { type: 'AuthState', values: ['connected'] }, { type: 'AuthScopes', values: ['codewhisperer:completions', 'codewhisperer:analysis'] },