diff --git a/chat-client/src/client/chat.test.ts b/chat-client/src/client/chat.test.ts index efd08da81d..2f6d8ad528 100644 --- a/chat-client/src/client/chat.test.ts +++ b/chat-client/src/client/chat.test.ts @@ -29,6 +29,7 @@ import { import { MynahUI } from '@aws/mynah-ui' import { TabFactory } from './tabs/tabFactory' import { ChatClientAdapter } from '../contracts/chatClientAdapter' +import { deprecationCard } from './texts/deprecation' describe('Chat', () => { const sandbox = sinon.createSandbox() @@ -74,11 +75,8 @@ describe('Chat', () => { window.removeEventListener('message', messageHandler as EventListener) messageHandler = undefined } + mynahUi.destroy() sandbox.restore() - - Object.keys(mynahUi.getAllTabs()).forEach(tabId => { - mynahUi.removeTab(tabId, (mynahUi as any).lastEventId) - }) }) after(() => { @@ -117,6 +115,19 @@ describe('Chat', () => { }) }) + it('hides the deprecation notice when it was previously acknowledged', () => { + mynahUi.destroy() + mynahUi = createChat(clientApi, { + agenticMode: true, + deprecationNoticeAcknowledged: true, + }) + + const tabId = mynahUi.getSelectedTabId() + const chatItems = tabId ? mynahUi.getTabData(tabId).getStore()?.chatItems : undefined + + assert.match(chatItems?.some(item => item.messageId === deprecationCard.messageId) ?? false, false) + }) + it('publishes telemetry event, when send to prompt is triggered', () => { const eventParams = { command: SEND_TO_PROMPT, params: { prompt: 'hey' } } const sendToPromptEvent = createInboundEvent(eventParams) @@ -397,6 +408,33 @@ describe('Chat', () => { }) describe('chatOptions', () => { + it('preserves the deprecation notice presentation when chat notifications are added', () => { + const chatOptionsRequest = createInboundEvent({ + command: CHAT_OPTIONS, + params: { + chatNotifications: [ + { + messageId: 'server-notification', + type: 'answer', + body: 'Server notification', + }, + ], + }, + }) + + window.dispatchEvent(chatOptionsRequest) + + const chatItems = mynahUi.getTabData(initialTabId).getStore()?.chatItems + const notice = chatItems?.find(item => item.messageId === deprecationCard.messageId) + + assert.match(chatItems?.[0].messageId, 'server-notification') + assert.match(notice?.title, deprecationCard.title) + assert.match(notice?.status, deprecationCard.status) + assert.match(notice?.border, deprecationCard.border) + assert.match(notice?.fullWidth, deprecationCard.fullWidth) + assert.match(notice?.canBeDismissed, deprecationCard.canBeDismissed) + }) + it('enables history and export features support', () => { const chatOptionsRequest = createInboundEvent({ command: CHAT_OPTIONS, @@ -432,6 +470,7 @@ describe('Chat', () => { it('enables MCP when params.mcpServers is true and config.agenticMode is true', function () { // Create a separate sandbox for this test const testSandbox = sinon.createSandbox() + let localMynahUi: MynahUI | undefined // Save original window functions const originalAddEventListener = window.addEventListener @@ -452,7 +491,7 @@ describe('Chat', () => { } // Create a new chat instance specifically for this test - const localMynahUi = createChat(localClientApi, { agenticMode: true }) + localMynahUi = createChat(localClientApi, { agenticMode: true }) // Create a new event const chatOptionsRequest = createInboundEvent({ @@ -474,6 +513,7 @@ describe('Chat', () => { // Restore window functions window.addEventListener = originalAddEventListener window.dispatchEvent = originalDispatchEvent + localMynahUi?.destroy() testSandbox.restore() } }) @@ -481,6 +521,7 @@ describe('Chat', () => { it('does not enable MCP when params.mcpServers is true but config.agenticMode is false', function () { // Create a separate sandbox for this test const testSandbox = sinon.createSandbox() + let localMynahUi: MynahUI | undefined // Save original window functions const originalAddEventListener = window.addEventListener @@ -501,7 +542,7 @@ describe('Chat', () => { } // Create a new chat instance specifically for this test - const localMynahUi = createChat(localClientApi, { agenticMode: false }) + localMynahUi = createChat(localClientApi, { agenticMode: false }) // Create a new event const chatOptionsRequest = createInboundEvent({ @@ -523,6 +564,7 @@ describe('Chat', () => { // Restore window functions window.addEventListener = originalAddEventListener window.dispatchEvent = originalDispatchEvent + localMynahUi?.destroy() testSandbox.restore() } }) @@ -530,6 +572,7 @@ describe('Chat', () => { it('does not enable MCP when params.mcpServers is false and config.agenticMode is true', function () { // Create a separate sandbox for this test const testSandbox = sinon.createSandbox() + let localMynahUi: MynahUI | undefined // Save original window functions const originalAddEventListener = window.addEventListener @@ -550,7 +593,7 @@ describe('Chat', () => { } // Create a new chat instance specifically for this test - const localMynahUi = createChat(localClientApi, { agenticMode: true }) + localMynahUi = createChat(localClientApi, { agenticMode: true }) // Create a new event const chatOptionsRequest = createInboundEvent({ @@ -572,6 +615,7 @@ describe('Chat', () => { // Restore window functions window.addEventListener = originalAddEventListener window.dispatchEvent = originalDispatchEvent + localMynahUi?.destroy() testSandbox.restore() } }) @@ -579,6 +623,7 @@ describe('Chat', () => { it('does not enable MCP when params.mcpServers is undefined and config.agenticMode is true', function () { // Create a separate sandbox for this test const testSandbox = sinon.createSandbox() + let localMynahUi: MynahUI | undefined // Save original window functions const originalAddEventListener = window.addEventListener @@ -599,7 +644,7 @@ describe('Chat', () => { } // Create a new chat instance specifically for this test - const localMynahUi = createChat(localClientApi, { agenticMode: true }) + localMynahUi = createChat(localClientApi, { agenticMode: true }) // Create a new event const chatOptionsRequest = createInboundEvent({ @@ -620,6 +665,7 @@ describe('Chat', () => { // Restore window functions window.addEventListener = originalAddEventListener window.dispatchEvent = originalDispatchEvent + localMynahUi?.destroy() testSandbox.restore() } }) @@ -675,6 +721,7 @@ describe('Chat', () => { handleMessageReceive: handleMessageReceiveStub, isSupportedTab: () => false, } + mynahUi.destroy() mynahUi = createChat( clientApi, { diff --git a/chat-client/src/client/chat.ts b/chat-client/src/client/chat.ts index bf52443679..b66e35b3ab 100644 --- a/chat-client/src/client/chat.ts +++ b/chat-client/src/client/chat.ts @@ -43,7 +43,6 @@ import { CONTEXT_COMMAND_NOTIFICATION_METHOD, CONVERSATION_CLICK_REQUEST_METHOD, CREATE_PROMPT_NOTIFICATION_METHOD, - ChatMessage, ChatOptionsUpdateParams, ChatParams, ChatUpdateParams, @@ -129,7 +128,9 @@ const getDefaultTabConfig = (agenticMode?: boolean) => { type ChatClientConfig = Pick & { disclaimerAcknowledged?: boolean + // Retained for compatibility with clients that still send the former feature-card state. pairProgrammingAcknowledged?: boolean + deprecationNoticeAcknowledged?: boolean agenticMode?: boolean modelSelectionEnabled?: boolean stringOverrides?: Partial @@ -368,7 +369,7 @@ export const createChat = ( // that tab does not have banner message, which arrives in ChatOptions above. const store = mynahUi.getTabData(tabFactory.initialTabId)?.getStore() || {} const chatItems = store.chatItems || [] - const updatedInitialItems = tabFactory.getChatItems(false, false, chatItems as ChatMessage[]) + const updatedInitialItems = [...tabFactory.getChatItems(false, false), ...chatItems] // First clear the tab, so that messages are not appended https://github.com/aws/mynah-ui/blob/38608dff905b3790d85c73e2911ec7071c8a8cdf/docs/USAGE.md#using-updatestore-function mynahUi.updateStore(tabFactory.initialTabId, { @@ -585,7 +586,7 @@ export const createChat = ( messager, tabFactory, config?.disclaimerAcknowledged ?? false, - config?.pairProgrammingAcknowledged ?? false, + config?.deprecationNoticeAcknowledged ?? false, chatClientAdapter, featureConfig, !!config?.agenticMode, diff --git a/chat-client/src/client/mynahUi.test.ts b/chat-client/src/client/mynahUi.test.ts index a69cf883c7..682f0cafd0 100644 --- a/chat-client/src/client/mynahUi.test.ts +++ b/chat-client/src/client/mynahUi.test.ts @@ -16,6 +16,7 @@ import { ChatClientAdapter } from '../contracts/chatClientAdapter' import { ChatMessage, ContextCommand, ListAvailableModelsResult } from '@aws/language-server-runtimes-types' import { ChatHistory } from './features/history' import { pairProgrammingModeOn, pairProgrammingModeOff } from './texts/pairProgramming' +import { deprecationCard } from './texts/deprecation' import { strictEqual } from 'assert' describe('MynahUI', () => { @@ -91,7 +92,7 @@ describe('MynahUI', () => { createTabStub.returns({}) getChatItemsStub = sinon.stub(tabFactory, 'getChatItems') getChatItemsStub.returns([]) - const mynahUiResult = createMynahUi(messager, tabFactory, true, true, undefined, undefined, true) + const mynahUiResult = createMynahUi(messager, tabFactory, true, false, undefined, undefined, true) mynahUi = mynahUiResult[0] inboundChatApi = mynahUiResult[1] getSelectedTabIdStub = sinon.stub(mynahUi, 'getSelectedTabId') @@ -104,11 +105,8 @@ describe('MynahUI', () => { }) afterEach(() => { + mynahUi.destroy() sinon.restore() - - Object.keys(mynahUi.getAllTabs()).forEach(tabId => { - mynahUi.removeTab(tabId, (mynahUi as any).lastEventId) - }) }) describe('handleChatPrompt', () => { @@ -163,14 +161,18 @@ describe('MynahUI', () => { }) describe('openTab', () => { - it('should create a new tab with welcome messages if tabId not passed and previous messages not passed', () => { + it('should show the deprecation card while initializing the first tab', () => { + sinon.assert.calledWith(getChatItemsStub, true, true) + }) + + it('should show the deprecation card in each new tab until it is acknowledged', () => { createTabStub.resetHistory() getChatItemsStub.resetHistory() inboundChatApi.openTab(requestId, {}) sinon.assert.calledOnceWithExactly(createTabStub, false) - sinon.assert.calledOnceWithExactly(getChatItemsStub, true, false, undefined) + sinon.assert.calledOnceWithExactly(getChatItemsStub, true, true, undefined) sinon.assert.notCalled(selectTabSpy) sinon.assert.calledOnce(onOpenTabSpy) }) @@ -206,6 +208,31 @@ describe('MynahUI', () => { sinon.assert.calledOnce(onOpenTabSpy) }) + it('should show the deprecation card in a new tab after a restored chat is opened', () => { + const mockMessages: ChatMessage[] = [ + { + messageId: 'restored-message', + body: 'Restored response', + type: ChatItemType.ANSWER, + }, + ] + + createTabStub.resetHistory() + getChatItemsStub.resetHistory() + + inboundChatApi.openTab(requestId, { + newTabOptions: { + data: { + messages: mockMessages, + }, + }, + }) + inboundChatApi.openTab(requestId, {}) + + sinon.assert.calledWithExactly(getChatItemsStub.firstCall, false, false, mockMessages) + sinon.assert.calledWithExactly(getChatItemsStub.secondCall, true, true, undefined) + }) + it('should call onOpenTab if a new tab if tabId not passed and tab not created', () => { createTabStub.resetHistory() getChatItemsStub.resetHistory() @@ -251,6 +278,7 @@ describe('MynahUI', () => { this.timeout(10000) // Increase timeout to 10 seconds // clear create tab stub since set up process calls it twice createTabStub.resetHistory() + getChatItemsStub.resetHistory() // Stub setTimeout to execute immediately const setTimeoutStub = sinon.stub(global, 'setTimeout').callsFake((fn: Function) => { fn() @@ -265,6 +293,7 @@ describe('MynahUI', () => { inboundChatApi.sendGenericCommand({ genericCommand, selection, tabId, triggerType }) sinon.assert.calledOnceWithExactly(createTabStub, false) + sinon.assert.calledOnceWithExactly(getChatItemsStub, true, true, []) // updateStore is called four times for a brand new tab: // 1. onTabAdd seeds the tab (chatItems + welcome tabHeaderDetails) // 2. handleChatPrompt clears the welcome splash before the first prompt @@ -766,14 +795,70 @@ describe('MynahUI', () => { stringOverrides ) - // Access the config texts from the instance - const configTexts = (customMynahUi as any).props.config.texts + try { + // Access the config texts from the instance + const configTexts = (customMynahUi as any).props.config.texts + + // Verify that string overrides were applied and defaults are preserved + strictEqual(configTexts.spinnerText, 'Custom loading message...') + strictEqual(configTexts.stopGenerating, 'Custom stop text') + strictEqual(configTexts.showMore, 'Custom show more text') + strictEqual(configTexts.clickFileToViewDiff, uiComponentsTexts.clickFileToViewDiff) + } finally { + customMynahUi.destroy() + } + }) + }) + + describe('onMessageDismiss', () => { + it('acknowledges the deprecation card and removes it from every open and future new chat', () => { + const updateTabDefaultsSpy = sinon.spy(mynahUi, 'updateTabDefaults') + const retainedItem = { + messageId: 'retained-message', + type: ChatItemType.ANSWER, + body: 'Keep me', + } + getAllTabsStub.returns({ + 'tab-1': { + store: { + chatItems: [deprecationCard, retainedItem], + }, + }, + 'tab-2': { + store: { + chatItems: [deprecationCard], + }, + }, + 'tab-3': { + store: { + chatItems: [retainedItem], + }, + }, + }) + updateStoreSpy.resetHistory() + ;(mynahUi as any).props.onMessageDismiss('tab-1', deprecationCard.messageId) + + sinon.assert.calledWithExactly( + outboundChatApi.chatPromptOptionAcknowledged as sinon.SinonStub, + deprecationCard.messageId + ) + sinon.assert.calledWithExactly(updateStoreSpy, 'tab-1', { chatItems: [retainedItem] }) + sinon.assert.calledWithExactly(updateStoreSpy, 'tab-2', { chatItems: [] }) + sinon.assert.neverCalledWith(updateStoreSpy, 'tab-3', sinon.match.any) + sinon.assert.calledWithExactly(getChatItemsStub, true, false) + sinon.assert.calledWithExactly(updateTabDefaultsSpy, { + store: { + chatItems: [], + }, + }) + + createTabStub.resetHistory() + getChatItemsStub.resetHistory() + + inboundChatApi.openTab(requestId, {}) - // Verify that string overrides were applied and defaults are preserved - strictEqual(configTexts.spinnerText, 'Custom loading message...') - strictEqual(configTexts.stopGenerating, 'Custom stop text') - strictEqual(configTexts.showMore, 'Custom show more text') - strictEqual(configTexts.clickFileToViewDiff, uiComponentsTexts.clickFileToViewDiff) + sinon.assert.calledOnceWithExactly(createTabStub, false) + sinon.assert.calledOnceWithExactly(getChatItemsStub, true, false, undefined) }) }) }) @@ -817,6 +902,11 @@ describe('withAdapter', () => { mynahUi = mynahUiResult[0] }) + afterEach(() => { + mynahUi.destroy() + sinon.restore() + }) + it('should instantiate and inject mynahUIRef to Adapter', () => { assert.match(mynahUIRef.mynahUI, mynahUi) }) diff --git a/chat-client/src/client/mynahUi.ts b/chat-client/src/client/mynahUi.ts index 5f24c71e45..0a8fc11a9a 100644 --- a/chat-client/src/client/mynahUi.ts +++ b/chat-client/src/client/mynahUi.ts @@ -69,7 +69,8 @@ import { toMynahIcon, } from './utils' import { ChatHistory, ChatHistoryList } from './features/history' -import { pairProgrammingModeOff, pairProgrammingModeOn, programmerModeCard } from './texts/pairProgramming' +import { pairProgrammingModeOff, pairProgrammingModeOn } from './texts/pairProgramming' +import { deprecationCard } from './texts/deprecation' import { ContextRule, RulesList } from './features/rules' import { getModelSelectionChatItem, modelUnavailableBanner, modelThrottledBanner } from './texts/modelSelection' import { getWelcomeTabHeader } from './texts/welcome' @@ -323,7 +324,7 @@ export const createMynahUi = ( messager: Messager, tabFactory: TabFactory, disclaimerAcknowledged: boolean, - pairProgrammingCardAcknowledged: boolean, + deprecationNoticeAcknowledged: boolean, customChatClientAdapter?: ChatClientAdapter, featureConfig?: Map, agenticMode?: boolean, @@ -331,7 +332,7 @@ export const createMynahUi = ( os?: string ): [MynahUI, InboundChatApi] => { let disclaimerCardActive = !disclaimerAcknowledged - let programmingModeCardActive = !pairProgrammingCardAcknowledged + let deprecationCardActive = !deprecationNoticeAcknowledged let contextCommandGroups: ContextCommandGroups | undefined let lastFilterTabId: string | undefined @@ -434,7 +435,7 @@ export const createMynahUi = ( // We check if tabMetadata.openTabKey exists - if it does and is set to true, we skip showing welcome messages // since this indicates we're loading a previous chat session rather than starting a new one. if (!tabStore?.tabMetadata || !tabStore.tabMetadata.openTabKey) { - defaultTabConfig.chatItems = tabFactory.getChatItems(true, programmingModeCardActive, []) + defaultTabConfig.chatItems = tabFactory.getChatItems(true, deprecationCardActive, []) // Roll a fresh "Did you know?" tip for every new tab. The // mynah-ui defaults.store is built once at startup, so without // this override every new tab would inherit the same cached tip. @@ -712,14 +713,24 @@ export const createMynahUi = ( messager.onPromptInputButtonClick(payload) }, onMessageDismiss: (tabId, messageId) => { - if (messageId === programmerModeCard.messageId) { - programmingModeCardActive = false + if (messageId === deprecationCard.messageId) { + deprecationCardActive = false messager.onChatPromptOptionAcknowledged(messageId) - // Update the tab defaults to hide the programmer mode card for new tabs + // Mynah removes the dismissed item from the active tab. Remove + // any remaining copies from the other open tabs as well. + Object.entries(mynahUi.getAllTabs()).forEach(([storeTabId, tabData]) => { + const chatItems = tabData.store?.chatItems + const updatedChatItems = chatItems?.filter(item => item.messageId !== deprecationCard.messageId) + if (updatedChatItems && updatedChatItems.length !== chatItems?.length) { + mynahUi.updateStore(storeTabId, { chatItems: updatedChatItems }) + } + }) + + // Update the tab defaults to hide the acknowledged card for new tabs. mynahUi.updateTabDefaults({ store: { - chatItems: tabFactory.getChatItems(true, false), + chatItems: tabFactory.getChatItems(true, deprecationCardActive), }, }) } @@ -823,7 +834,7 @@ export const createMynahUi = ( isSelected: true, store: { ...tabFactory.createTab(disclaimerCardActive), - chatItems: tabFactory.getChatItems(true, programmingModeCardActive), + chatItems: tabFactory.getChatItems(true, deprecationCardActive), }, }, }, @@ -1403,8 +1414,13 @@ ${params.message}`, const messages = params.newTabOptions?.data?.messages const tabId = createTabId(true) if (tabId) { + const needWelcomeMessages = !messages mynahUi.updateStore(tabId, { - chatItems: tabFactory.getChatItems(messages ? false : true, programmingModeCardActive, messages), + chatItems: tabFactory.getChatItems( + needWelcomeMessages, + needWelcomeMessages && deprecationCardActive, + messages + ), // onTabAdd suppresses the welcome splash whenever // openTabKey is true (which createTabId(true) sets), so // re-establish it here for the no-messages case so a diff --git a/chat-client/src/client/tabs/tabFactory.test.ts b/chat-client/src/client/tabs/tabFactory.test.ts index 815e81a22e..5a13fddf07 100644 --- a/chat-client/src/client/tabs/tabFactory.test.ts +++ b/chat-client/src/client/tabs/tabFactory.test.ts @@ -3,6 +3,8 @@ import { TabFactory } from './tabFactory' import * as assert from 'assert' import { pairProgrammingPromptInput } from '../texts/pairProgramming' import { modelSelection } from '../texts/modelSelection' +import { deprecationCard } from '../texts/deprecation' +import { ChatMessage } from '@aws/language-server-runtimes-types' describe('tabFactory', () => { describe('getDefaultTabData', () => { @@ -121,4 +123,51 @@ describe('tabFactory', () => { assert.deepStrictEqual(result.promptInputOptions, []) }) }) + + describe('getChatItems', () => { + it('shows the deprecation card in a new chat when it is active', () => { + const tabFactory = new TabFactory({}) + + const result = tabFactory.getChatItems(true, true) + + assert.deepStrictEqual(result, [deprecationCard]) + }) + + it('replaces the agentic feature card in agentic mode', () => { + const tabFactory = new TabFactory({}) + tabFactory.enableAgenticMode() + + const result = tabFactory.getChatItems(true, true) + + assert.deepStrictEqual(result, [deprecationCard]) + }) + + it('hides the deprecation card after it has been acknowledged', () => { + const tabFactory = new TabFactory({}) + tabFactory.enableAgenticMode() + + const result = tabFactory.getChatItems(true, false) + + assert.deepStrictEqual(result, []) + }) + + it('does not add welcome cards to restored chats', () => { + const messages: ChatMessage[] = [ + { + body: 'Restored response', + type: 'answer', + }, + ] + const tabFactory = new TabFactory({}) + + const result = tabFactory.getChatItems(false, true, messages) + + assert.equal(result.length, 1) + assert.equal(result[0].body, 'Restored response') + assert.equal( + result.some(item => item.messageId === deprecationCard.messageId), + false + ) + }) + }) }) diff --git a/chat-client/src/client/tabs/tabFactory.ts b/chat-client/src/client/tabs/tabFactory.ts index 9414fc49ff..8085fe2411 100644 --- a/chat-client/src/client/tabs/tabFactory.ts +++ b/chat-client/src/client/tabs/tabFactory.ts @@ -9,10 +9,11 @@ import { import { disclaimerCard } from '../texts/disclaimer' import { ChatMessage } from '@aws/language-server-runtimes-types' import { ChatHistory } from '../features/history' -import { pairProgrammingPromptInput, programmerModeCard } from '../texts/pairProgramming' +import { pairProgrammingPromptInput } from '../texts/pairProgramming' import { modelSelection } from '../texts/modelSelection' import { getWelcomeTabHeader } from '../texts/welcome' import { chatMessageToChatItem } from '../utils' +import { deprecationCard } from '../texts/deprecation' export type DefaultTabData = MynahUIDataModel @@ -63,14 +64,14 @@ export class TabFactory { public getChatItems( needWelcomeMessages: boolean, - pairProgrammingCardActive: boolean, + deprecationCardActive: boolean, chatMessages?: ChatMessage[] ): ChatItem[] { return [ ...(this.bannerMessage ? [this.getBannerMessage() as ChatItem] : []), ...(needWelcomeMessages - ? this.agenticMode && pairProgrammingCardActive - ? [programmerModeCard] + ? deprecationCardActive + ? [deprecationCard] : [] : chatMessages ? chatMessages.map(msg => chatMessageToChatItem(msg, this.agenticMode)) diff --git a/chat-client/src/client/texts/deprecation.test.ts b/chat-client/src/client/texts/deprecation.test.ts new file mode 100644 index 0000000000..ffb5d6a1a6 --- /dev/null +++ b/chat-client/src/client/texts/deprecation.test.ts @@ -0,0 +1,22 @@ +import * as assert from 'assert' +import { ChatItemType } from '@aws/mynah-ui' +import { deprecationCard } from './deprecation' + +describe('deprecationCard', () => { + it('uses the approved copy and warning presentation', () => { + assert.equal(deprecationCard.type, ChatItemType.ANSWER) + assert.equal(deprecationCard.messageId, 'client-deprecation-notice') + assert.equal(deprecationCard.title, 'IMPORTANT') + assert.equal(deprecationCard.status, 'warning') + assert.equal(deprecationCard.border, true) + assert.equal(deprecationCard.fullWidth, true) + assert.equal(deprecationCard.canBeDismissed, true) + assert.equal(deprecationCard.header?.icon, 'warning') + assert.equal(deprecationCard.header?.iconStatus, 'warning') + assert.equal(deprecationCard.header?.body, '### Amazon Q Developer IDE plugins: end of support') + assert.equal( + deprecationCard.body, + 'On April 30, 2027, AWS will discontinue support for Amazon Q Developer IDE plugins. For capabilities similar to Amazon Q Developer IDE plugins, [explore Kiro](https://kiro.dev) to access the latest models and features, including agentic coding, chat and MCP support.\n\n[Learn more](https://aws.amazon.com/blogs/devops/amazon-q-developer-end-of-support-announcement/)' + ) + }) +}) diff --git a/chat-client/src/client/texts/deprecation.ts b/chat-client/src/client/texts/deprecation.ts new file mode 100644 index 0000000000..00fb3ffcd7 --- /dev/null +++ b/chat-client/src/client/texts/deprecation.ts @@ -0,0 +1,17 @@ +import { ChatItem, ChatItemType } from '@aws/mynah-ui' + +export const deprecationCard: ChatItem = { + type: ChatItemType.ANSWER, + messageId: 'client-deprecation-notice', + title: 'IMPORTANT', + status: 'warning', + border: true, + fullWidth: true, + canBeDismissed: true, + header: { + icon: 'warning', + iconStatus: 'warning', + body: '### Amazon Q Developer IDE plugins: end of support', + }, + body: 'On April 30, 2027, AWS will discontinue support for Amazon Q Developer IDE plugins. For capabilities similar to Amazon Q Developer IDE plugins, [explore Kiro](https://kiro.dev) to access the latest models and features, including agentic coding, chat and MCP support.\n\n[Learn more](https://aws.amazon.com/blogs/devops/amazon-q-developer-end-of-support-announcement/)', +} diff --git a/chat-client/src/client/texts/pairProgramming.test.ts b/chat-client/src/client/texts/pairProgramming.test.ts index 8181f50c8b..5ccb6d7980 100644 --- a/chat-client/src/client/texts/pairProgramming.test.ts +++ b/chat-client/src/client/texts/pairProgramming.test.ts @@ -1,26 +1,8 @@ import * as assert from 'assert' import { ChatItemType } from '@aws/mynah-ui' -import { - programmerModeCard, - pairProgrammingPromptInput, - pairProgrammingModeOn, - pairProgrammingModeOff, -} from './pairProgramming' +import { pairProgrammingPromptInput, pairProgrammingModeOn, pairProgrammingModeOff } from './pairProgramming' describe('pairProgramming', () => { - describe('programmerModeCard', () => { - it('has correct properties', () => { - assert.equal(programmerModeCard.type, ChatItemType.ANSWER) - assert.equal(programmerModeCard.title, 'NEW FEATURE') - assert.equal(programmerModeCard.messageId, 'programmerModeCardId') - assert.equal(programmerModeCard.fullWidth, true) - assert.equal(programmerModeCard.canBeDismissed, true) - assert.ok(programmerModeCard.body?.includes('Amazon Q can now help')) - assert.equal(programmerModeCard.header?.icon, 'code-block') - assert.equal(programmerModeCard.header?.iconStatus, 'primary') - }) - }) - describe('pairProgrammingPromptInput', () => { it('has correct properties', () => { assert.equal(pairProgrammingPromptInput.type, 'switch') diff --git a/chat-client/src/client/texts/pairProgramming.ts b/chat-client/src/client/texts/pairProgramming.ts index 335a37069c..a92d4bf89f 100644 --- a/chat-client/src/client/texts/pairProgramming.ts +++ b/chat-client/src/client/texts/pairProgramming.ts @@ -1,19 +1,5 @@ import { ChatItem, ChatItemFormItem, ChatItemType } from '@aws/mynah-ui' -export const programmerModeCard: ChatItem = { - type: ChatItemType.ANSWER, - title: 'NEW FEATURE', - header: { - icon: 'code-block', - iconStatus: 'primary', - body: '### An interactive, agentic coding experience', - }, - messageId: 'programmerModeCardId', - fullWidth: true, - canBeDismissed: true, - body: 'Amazon Q can now help you write, modify, and maintain code by combining the power of natural language understanding with the ability to take actions on your behalf such as directly making code changes, modifying files, and running commands.', -} - export const pairProgrammingPromptInput: ChatItemFormItem = { type: 'switch', id: 'pair-programmer-mode', diff --git a/chat-client/src/test/jsDomInjector.ts b/chat-client/src/test/jsDomInjector.ts index b73ad67484..96288daff4 100644 --- a/chat-client/src/test/jsDomInjector.ts +++ b/chat-client/src/test/jsDomInjector.ts @@ -11,6 +11,8 @@ export function injectJSDOM() { global.window = dom.window as unknown as Window & typeof globalThis global.document = dom.window.document global.self = dom.window as unknown as Window & typeof globalThis + // MynahUI.destroy() exercises browser DOM cleanup that expects Node to be global. + global.Node = dom.window.Node global.Element = dom.window.Element global.HTMLElement = dom.window.HTMLElement global.CustomEvent = dom.window.CustomEvent diff --git a/server/aws-lsp-codewhisperer/src/language-server/netTransform/atxTransformHandler.ts b/server/aws-lsp-codewhisperer/src/language-server/netTransform/atxTransformHandler.ts index f7e569f669..35f7b52863 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/netTransform/atxTransformHandler.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/netTransform/atxTransformHandler.ts @@ -4,6 +4,7 @@ import * as archiver from 'archiver' import got from 'got' import * as path from 'path' import * as crypto from 'crypto' +import { pipeline } from 'stream/promises' import { NodeHttpHandler } from '@smithy/node-http-handler' import AdmZip = require('adm-zip') import { ArtifactManager } from './artifactManager' @@ -115,6 +116,29 @@ interface BeamedRepoInfo { // Bounds for the beam-map candidate scan (serial download+parse per candidate). const BEAM_MAP_SCAN_MAX_CANDIDATES = 25 const BEAM_MAP_SCAN_BUDGET_MS = 15000 +// How long to stop starting optional beam-map scans after FES throttles us. Deliberately longer than +// the 30s dashboard/info-bar poll interval: a cooldown shorter than the poll would merely delay calls +// within one sweep instead of dropping whole sweeps, which is the only thing that reduces our rate. +const BEAM_THROTTLE_COOLDOWN_MS = 90000 +// Cap on the parsed-artifact cache. The two bounds above limit ONE scan; they cannot +// limit the number of scans, so repeated discovery sweeps re-downloaded the same +// artifacts indefinitely. The cache is what bounds the total. +// +// 4000 is derived from a measurement, not chosen: a 25-minute session on a 32-job +// workspace touched 1043 DISTINCT artifacts while issuing ~30000 downloads (~29x +// redundancy). An earlier value of 500 sat BELOW that working set, so the cache filled, +// evicted, and refilled continuously — 36 evictions in 25 minutes — and the flood did +// not move at all. The cap must exceed the working set or the cache is worse than +// useless: it pays the bookkeeping and delivers no hits. +const BEAM_JSON_ARTIFACT_CACHE_MAX = 4000 + +// How long a PARSED artifact stays cached. Negatives ("not JSON") never expire — that is a property +// of the bytes — but a positive can be rewritten under the same artifactId (web-orc re-writes the +// beam-map), so pinning one for the session would hide a repo beamed later. 60s is far longer than +// the ~13s sweep cadence, so it still collapses the repeated fetches this cache exists to stop, +// while bounding staleness to one minute. It also bounds RETENTION: parsed values are arbitrary +// size, so a TTL keeps customer artifact content from living in the LSP for the whole session. +const BEAM_JSON_POSITIVE_TTL_MS = 60_000 /** * ATX Transform Handler - Business logic for ATX FES Transform operations @@ -142,6 +166,35 @@ export class ATXTransformHandler { // (surface for auto-approve). private jobsPastLocalBuild: Set = new Set() + // Beam: parsed small-JSON artifacts keyed by artifactId. A `null` value is a positive result + // meaning "these bytes are definitively not JSON" — the case that dominated the waste, and the + // only one cached indefinitely. Parsed values carry an expiry because the same id CAN be + // rewritten (see downloadJsonArtifact for why immutability is not assumed). Transient failures + // — no presigned URL — are never cached at all. + private readonly beamJsonArtifactCache = new Map() + // Latch so the cap-reached notice is logged once per process, not once per eviction. + private beamJsonArtifactCacheEvicted = false + + // Process-wide throttle cooldown. The per-call retry in createArtifactDownloadUrl is resilience + // for ONE call and does not reduce our rate — under sustained throttling it raises it, because a + // rejected call becomes up to four. Nothing currently tells the client to stop STARTING work. + // That is the gap behind an alarm that has fired five times in 2026 without resolution: each + // round produced a local "make this cheaper" fix while the client kept initiating at the same + // rate no matter how hard the service pushed back. + // + // So: any throttle sets a cooldown, and the optional beam-map scan — the only unbounded-ish + // fan-out we own — skips while it is in force. Deliberately longer than the 30s poll interval so + // it actually drops sweeps rather than merely delaying calls within one. + // + // Skipping is SAFE and not a feature regression: the beam-map is optional enrichment. When it is + // absent, discovery already derives beamed repos from beam-status artifacts (observed live: + // "no beam-map for job=… — derived 1 beamed repo(s) from beam-status artifacts"), and + // isRepoLbvOpen defaults to OPEN on a fetch failure so nothing is ever wrongly hidden. A + // throttled sweep therefore degrades to the path that already works, and the next uncooled sweep + // picks the beam-map up. + private beamThrottleCooldownUntilMs = 0 + private beamThrottleCooldownLogged = false + constructor(serviceManager: AtxTokenServiceManager, workspace: Workspace, logging: Logging, runtime: Runtime) { this.serviceManager = serviceManager this.workspace = workspace @@ -686,7 +739,23 @@ export class ATXTransformHandler { // job (logs, metadata) could otherwise stall the IDE's discovery UI. Cap the number // scanned and enforce an overall deadline; the beam-map is written early and is // small, so a bounded scan reliably finds it. - const candidates = allCandidates.slice(0, BEAM_MAP_SCAN_MAX_CANDIDATES) + // Throttle cooldown: skip the optional scan rather than adding to the pressure. The + // existing per-call retry only rescues a single call; this is what stops us STARTING more + // work while the service is rejecting us. + // + // Empty the candidate list rather than returning early — an early return would abandon + // the whole of listBeamedRepos, including the beam-status discovery below, and the Beamed + // tab would go empty for the duration of the cooldown. Emptying skips only the optional + // enrichment and lets the path that actually finds beamed repos run untouched. + const beamThrottleCoolingDown = this.isBeamThrottleCooldownActive() + if (beamThrottleCoolingDown) { + this.logging.log( + `[BEAM-PKG] beam-map scan: SKIPPED — throttle cooldown active for another ` + + `${Math.max(0, this.beamThrottleCooldownUntilMs - Date.now())}ms. ` + + `Beam-status discovery continues; the beam-map is optional enrichment.` + ) + } + const candidates = beamThrottleCoolingDown ? [] : allCandidates.slice(0, BEAM_MAP_SCAN_MAX_CANDIDATES) if (allCandidates.length > candidates.length) { this.logging.log( `[BEAM-PKG] beam-map scan: capping ${allCandidates.length} candidate(s) to ${candidates.length}` @@ -778,7 +847,7 @@ export class ATXTransformHandler { `[BEAM-PKG] beam-map repo '${nr.RepositoryName}': no transformed zip matched — falling back to beam-map artifactId ${nr.BeamArtifactId} (may not be downloadable)` ) } - const lbvOpen = planRoot ? this.isRepoLbvOpen(planRoot, nr.RepositoryName) : true + const lbvOpen = planRoot ? this.isRepoLbvOpen(planRoot, nr.RepositoryName, parentJobId) : true const lbvPending = planRoot ? this.isRepoLbvHitlPending(planRoot, nr.RepositoryName) : true beamed.push({ RepositoryName: nr.RepositoryName, @@ -790,7 +859,7 @@ export class ATXTransformHandler { IsLbvPending: lbvPending, }) this.logging.log( - `[BEAM-PKG] beamed repo (from beam-map) | repo=${nr.RepositoryName} artifact=${artifactId} stepId=${nr.BeamStepId || ''} scenario=${nr.BeamScenario || 'transformed'} lbvOpen=${lbvOpen} lbvPending=${lbvPending}` + `[BEAM-PKG] beamed repo (from beam-map) | job=${parentJobId} repo=${nr.RepositoryName} artifact=${artifactId} stepId=${nr.BeamStepId || ''} scenario=${nr.BeamScenario || 'transformed'} lbvOpen=${lbvOpen} lbvPending=${lbvPending}` ) } } else { @@ -834,7 +903,7 @@ export class ATXTransformHandler { ) continue } - const lbvOpen = planRoot ? this.isRepoLbvOpen(planRoot, repoName) : true + const lbvOpen = planRoot ? this.isRepoLbvOpen(planRoot, repoName, parentJobId) : true const lbvPending = planRoot ? this.isRepoLbvHitlPending(planRoot, repoName) : true beamed.push({ RepositoryName: repoName, @@ -846,7 +915,7 @@ export class ATXTransformHandler { IsLbvPending: lbvPending, }) this.logging.log( - `[BEAM-PKG] beamed repo (from beam-status) | repo=${repoName} artifact=${zip.artifactId} path=${zip.path} lbvOpen=${lbvOpen} lbvPending=${lbvPending}` + `[BEAM-PKG] beamed repo (from beam-status) | job=${parentJobId} repo=${repoName} artifact=${zip.artifactId} path=${zip.path} lbvOpen=${lbvOpen} lbvPending=${lbvPending}` ) } this.logging.log( @@ -927,9 +996,59 @@ export class ATXTransformHandler { * "Unexpected token 'P'"). So: fetch as a buffer, try JSON.parse first, and on * failure unzip (AdmZip) and parse the first JSON entry inside. */ + /** + * Record that FES pushed back. Set on every throttle, retried or not — the signal is "we are + * being rejected", which is equally true on the first attempt and the last. + */ + private noteBeamThrottled(): void { + this.beamThrottleCooldownUntilMs = Date.now() + BEAM_THROTTLE_COOLDOWN_MS + if (!this.beamThrottleCooldownLogged) { + this.beamThrottleCooldownLogged = true + this.logging.log( + `[BEAM-PKG] FES throttled us — pausing the optional beam-map scan for ` + + `${BEAM_THROTTLE_COOLDOWN_MS}ms after each throttle. Logged once per process.` + ) + } + } + + private isBeamThrottleCooldownActive(): boolean { + return Date.now() < this.beamThrottleCooldownUntilMs + } + private async downloadJsonArtifact(workspaceId: string, jobId: string, artifactId: string): Promise { + // Beam discovery re-scans the same candidate artifacts on every sweep, so without a cache + // the same artifact is downloaded and re-parsed on every pass (measured: ~2000 download-URL + // creations and 1230 repeated "not JSON" parses in 90s, which provoked AccessDenied on + // unrelated calls — chat included). + // + // Deliberately NOT relying on artifacts being immutable. An earlier version did, on the + // assumption that a re-upload mints a new artifactId — but this file's own comments record + // web-orc RE-WRITING the beam-map (see the beam-map notes above: "re-written on each beam; + // latest wins", and the BeamArtifactId "thrash" that forced the beam-status fallback). If a + // rewrite reuses the id, a permanent positive cache would pin the FIRST beam-map for the + // whole LSP session, so a repo beamed later never appears in the panel until restart, and a + // re-beam would leave chat routed at a dead stepId. + // + // So: NEGATIVES are cached permanently (an artifact that is not JSON does not become JSON, + // and negatives were 1230 of the 1980 wasted fetches — the bulk of the win), while POSITIVES + // get a short TTL. Positives are a handful per job, so re-fetching one every TTL is cheap, + // and it removes the dependency on platform semantics we have not verified. + const hit = this.beamJsonArtifactCache.get(artifactId) + if (hit && Date.now() < hit.expiresAt) { + // Re-insert to move this entry to the end: Map iterates in insertion order, so + // "oldest key first" is only a true LRU ordering if a hit refreshes position. + this.beamJsonArtifactCache.delete(artifactId) + this.beamJsonArtifactCache.set(artifactId, hit) + return hit.value + } + if (hit) { + // Expired positive — drop it so the fetch below refreshes it. + this.beamJsonArtifactCache.delete(artifactId) + } try { const dl = await this.createArtifactDownloadUrl(workspaceId, jobId, artifactId) + // No presigned URL is a transient condition (throttle/authz), so it is NOT + // cached — the next sweep must be free to retry. if (!dl?.s3PresignedUrl) return null const response = await got.get(dl.s3PresignedUrl, { headers: dl.requestHeaders || {}, @@ -940,7 +1059,9 @@ export class ATXTransformHandler { // Direct JSON first (artifact stored raw). try { - return JSON.parse(buf.toString('utf8')) + const parsed = JSON.parse(buf.toString('utf8')) + this.cacheJsonArtifact(artifactId, parsed) + return parsed } catch { // Not raw JSON — likely a ZIP (PK magic). Unzip and parse the first // JSON entry (the beam-map is a single JSON file inside). @@ -953,20 +1074,66 @@ export class ATXTransformHandler { this.logging.log( `[BEAM-PKG] downloadJsonArtifact: parsed ${artifactId} from zip entry ${entry.entryName}` ) + this.cacheJsonArtifact(artifactId, parsed) return parsed } catch { // not this entry — try the next } } } - throw new Error('artifact is neither raw JSON nor a zip containing JSON') + // Definitively not JSON, and not a zip containing JSON. That is a + // PERMANENT property of these bytes, so remember it: this was the bulk + // of the waste (1230 repeats in 90s), and it now logs once per artifact + // instead of once per sweep. + this.logging.log( + `ATX: downloadJsonArtifact: ${artifactId} is neither raw JSON nor a zip containing JSON — caching as not-JSON` + ) + this.cacheJsonArtifact(artifactId, null) + return null } } catch (error) { + // TRANSIENT (download/network/throttle/AccessDenied, or a malformed zip that + // made AdmZip throw). Deliberately NOT cached — caching one of these would + // permanently mark a readable artifact as unreadable. this.logging.log(`ATX: downloadJsonArtifact parse/download failed for ${artifactId}: ${String(error)}`) return null } } + /** + * Record a parsed artifact (or `null` for "definitively not JSON") against its + * immutable artifactId, evicting least-recently-used entries at the cap. + * + * Evicts ONE entry rather than clearing the map. The first version cleared wholesale, + * which turns an under-sized cap from a partial loss into a total one: at the cap it + * discards every entry including the ones about to be hit, so the hit rate collapses + * to zero and the cache does nothing. That is exactly what happened with a cap of 500 + * against a 1043-artifact working set. Single-victim eviction degrades gradually + * instead, so a workspace larger than the cap still gets most of the benefit. + */ + private cacheJsonArtifact(artifactId: string, value: any | null): void { + while (this.beamJsonArtifactCache.size >= BEAM_JSON_ARTIFACT_CACHE_MAX) { + const oldest = this.beamJsonArtifactCache.keys().next() + if (oldest.done) break + this.beamJsonArtifactCache.delete(oldest.value) + // Logged ONCE, not per eviction — the signal we want is "the working set + // exceeded the cap", which is what made the previous under-sized cap + // diagnosable. Per-eviction logging would itself become the flood. + if (!this.beamJsonArtifactCacheEvicted) { + this.beamJsonArtifactCacheEvicted = true + this.logging.log( + `[BEAM-PKG] downloadJsonArtifact cache hit its ${BEAM_JSON_ARTIFACT_CACHE_MAX}-entry cap — ` + + `evicting LRU. If this appears, the workspace's artifact working set exceeds the cap ` + + `and the hit rate is degrading; consider raising it.` + ) + } + } + // value === null means "definitively not JSON" — a permanent property of the bytes, so it + // never expires. A parsed positive may be rewritten under the same id, so it does. + const expiresAt = value === null ? Number.MAX_SAFE_INTEGER : Date.now() + BEAM_JSON_POSITIVE_TTL_MS + this.beamJsonArtifactCache.set(artifactId, { value, expiresAt }) + } + /** * Beam to IDE — download the beam artifact (source + beam-flag + context zip) * from the parent web job and extract it locally so the IDE can open the repo. @@ -1344,6 +1511,11 @@ export class ATXTransformHandler { name === 'ThrottlingException' || name === 'TooManyRequestsException' || /throttl|too many requests|rate exceeded/i.test(msg) + if (isThrottle) { + // Record it regardless of whether we retry: the signal is "the service is + // pushing back", which is true on the last attempt as much as the first. + this.noteBeamThrottled() + } if (isThrottle && attempt < maxAttempts) { // Exponential base (250, 500, 1000ms) with jitter: under concurrent // throttling, unjittered lockstep retries amplify the thundering herd that @@ -2381,25 +2553,43 @@ export class ATXTransformHandler { * name, then inspect its LBV descendant: OPEN = the LBV node exists and is non-terminal * (PENDING_HUMAN_INPUT / IN_PROGRESS / NOT_STARTED); CLOSED = terminal (SUCCEEDED / COMPLETED / * FAILED / STOPPED). Returns TRUE (open) when we can't resolve it — no plan yet, node not - * found, or no LBV child — so a freshly transferred repo is never hidden before its LBV status - * is known (the IDE mirrors this default). The caller skips this call entirely when no plan is - * available. + * found, or no LBV child on a still-live repo node — so a freshly transferred repo is never + * hidden before its LBV status is known (the IDE mirrors this default). The one exception is a + * repo node that is itself terminal with no LBV child: that beam ended without ever opening + * LBV, so it is CLOSED rather than forever-pending. The caller skips this call entirely when no + * plan is available. */ - private isRepoLbvOpen(planRoot: AtxPlanStep, repoName: string): boolean { + private isRepoLbvOpen(planRoot: AtxPlanStep, repoName: string, jobId: string): boolean { const repoNode = this.findBeamedRepoNode(planRoot, repoName) if (!repoNode) { // unknown → default open (don't hide a fresh transfer) this.logging.log( - `[BEAM-LBVOPEN] repo='${repoName}' → OPEN (default): no beamed repo node found in plan tree` + `[BEAM-LBVOPEN] job=${jobId} repo='${repoName}' → OPEN (default): no beamed repo node found in plan tree` ) return true } const lbvNode = this.findLbvNode(repoNode) if (!lbvNode) { + // No LBV child. Two very different states look identical here: a fresh transfer whose + // sub-agent hasn't created the HITL YET, and a beam whose sub-agent never booted and was + // reaped by the web orchestrator's spawn-gap recovery (which stops the instance and marks + // this repo node terminal). Only the repo node's own status separates them — so read it + // before defaulting to open, or a reaped repo sits in the Transferred list forever with a + // disabled Load button and no explanation (its re-beam notice goes to the job owner, not + // to whoever is waiting in the IDE). Safe on re-beam: the orchestrator re-drives this + // node to IN_PROGRESS before reusing it, so a re-beamed repo reads live again. + if (this.isTerminalStepStatus(repoNode.Status)) { + this.logging.log( + `[BEAM-LBVOPEN] job=${jobId} repo='${repoName}' node='${repoNode.StepName}' ` + + `repoStatus=${repoNode.Status} → CLOSED: no Local Build Verification child and the repo node is ` + + `terminal, so the beam ended without ever opening LBV (spawn-gap reap or cancel)` + ) + return false + } // no LBV node yet → still pending this.logging.log( - `[BEAM-LBVOPEN] repo='${repoName}' → OPEN (default): repo node '${repoNode.StepName}' has no Local Build Verification child yet` + `[BEAM-LBVOPEN] job=${jobId} repo='${repoName}' → OPEN (default): repo node '${repoNode.StepName}' has no Local Build Verification child yet` ) return true } @@ -2409,7 +2599,7 @@ export class ATXTransformHandler { // is how we PROVE the show-set actually flips: watch a repo go lbvStatus=IN_PROGRESS/ // PENDING_HUMAN_INPUT (OPEN) → SUCCEEDED (CLOSED) after Load, and confirm it then drops off. this.logging.log( - `[BEAM-LBVOPEN] repo='${repoName}' node='${repoNode.StepName}' lbvNode='${lbvNode.StepName}' ` + + `[BEAM-LBVOPEN] job=${jobId} repo='${repoName}' node='${repoNode.StepName}' lbvNode='${lbvNode.StepName}' ` + `lbvStatus=${lbvNode.Status} terminal=${terminal} → ${terminal ? 'CLOSED (will be hidden)' : 'OPEN (will show)'}` ) return !terminal @@ -2438,28 +2628,71 @@ export class ATXTransformHandler { * jobs that name the beam node just "") if no "(beamed)" node exists anywhere. */ private findBeamedRepoNode(step: AtxPlanStep, repoName: string): AtxPlanStep | null { - const beamed = this.findNodeByName(step, `${repoName} (beamed)`.toLowerCase()) + const beamed = this.pickLiveNode(this.findAllNodesByName(step, `${repoName} (beamed)`.toLowerCase())) if (beamed) return beamed - return this.findNodeByName(step, repoName.toLowerCase()) + return this.pickLiveNode(this.findAllNodesByName(step, repoName.toLowerCase())) } - private findNodeByName(step: AtxPlanStep, lowerName: string): AtxPlanStep | null { - if ((step.StepName || '').toLowerCase() === lowerName) return step + /** + * ALL name matches in DFS order, not just the first. Re-beam makes duplicates real: web-orc + * mints a fresh beam-batch parent per batch, so a repo beamed twice has TWO " (beamed)" + * nodes, and runtime RECREATES the LBV child rather than reviving the terminal one (only + * non-terminal children are reused). A first-match walk then resolves to the DEAD node, the + * repo reports lbvOpen=false, and a successful re-beam never reappears in the Beamed tab. + * A matched node's own subtree is not searched, matching the previous first-match semantics. + */ + private findAllNodesByName(step: AtxPlanStep, lowerName: string): AtxPlanStep[] { + const found: AtxPlanStep[] = [] + if ((step.StepName || '').toLowerCase() === lowerName) { + found.push(step) + return found + } for (const child of step.Children) { - const found = this.findNodeByName(child, lowerName) - if (found) return found + found.push(...this.findAllNodesByName(child, lowerName)) } - return null + return found + } + + /** + * Of several candidate nodes for one repo, pick the LIVE one: the first whose LBV child is + * still non-terminal. A candidate with no LBV child yet counts as live — it is pre-terminal, + * the same default isRepoLbvOpen applies elsewhere. When every candidate is terminal (all + * attempts finished) fall back to the LAST, which is the newest since children are appended. + * Non-terminal is the PRIMARY rule precisely so append-order is only ever a tiebreak. + */ + private pickLiveNode(candidates: AtxPlanStep[]): AtxPlanStep | null { + if (candidates.length === 0) return null + if (candidates.length === 1) return candidates[0] + const live = candidates.find(c => { + const lbv = this.findLbvNode(c) + return lbv ? !this.isTerminalStepStatus(lbv.Status) : true + }) + return live ?? candidates[candidates.length - 1] } - /** Find the "Local Build Verification" node anywhere under the given node. */ + /** + * Find the repo's LIVE "Local Build Verification" node. Prefers a non-terminal node over a + * terminal one for the same reason as pickLiveNode: after a re-beam the old STOPPED/FAILED + * child and the fresh one sit under the same parent, and a first-match walk picks the dead + * one. Falls back to the last (newest) when every attempt is terminal, which is what the + * terminal-drop in RebuildBeamedRepos wants to see. + */ private findLbvNode(step: AtxPlanStep): AtxPlanStep | null { - if ((step.StepName || '').toLowerCase().includes('local build verification')) return step + const all = this.findAllLbvNodes(step) + if (all.length === 0) return null + return all.find(n => !this.isTerminalStepStatus(n.Status)) ?? all[all.length - 1] + } + + private findAllLbvNodes(step: AtxPlanStep): AtxPlanStep[] { + const found: AtxPlanStep[] = [] + if ((step.StepName || '').toLowerCase().includes('local build verification')) { + found.push(step) + return found + } for (const child of step.Children) { - const found = this.findLbvNode(child) - if (found) return found + found.push(...this.findAllLbvNodes(child)) } - return null + return found } /** Terminal step statuses (LBV done, one way or another). */ @@ -4576,8 +4809,23 @@ export class ATXTransformHandler { const sendResult = (await this.atxClient!.send(command)) as any const sentMessageId = sendResult?.message?.messageId + // Join key for correlating a beamed turn across all three packages on one timeline. + // The two backends key on different fields — runtime on message_id_sent, web-orc on + // msg_len (its send response carries no createdAt) — so emit both names here rather + // than forcing either side to translate. beamStep identifies which repo's lane. + this.logging.log( + `[BEAM-CHAT] sent | message_id_sent=${sentMessageId ?? ''} msg_len=${messageText.length} ` + + `beamStep=${request.beamStepId || ''} job=${request.jobId ?? ''} ` + + `skipPolling=${!!request.skipPolling}` + ) + if (!sentMessageId || request.skipPolling) { - return { success: true, data: sendResult } + // Must match the shape the polling path returns below. The client reads the sent id + // from data.sentMessage.messageId and adds it to its seen-set so its own poll does + // not re-render the message the user just sent. Returning the raw sendResult nests + // that id under `message` instead of `sentMessage`, leaving it unreadable — so every + // message sent on this path rendered twice: once locally, once again from the poll. + return { success: true, data: { sentMessage: sendResult?.message, response: null } } } // Poll for response until the configured attempt ceiling (default 180 x 5s @@ -4737,26 +4985,57 @@ export class ATXTransformHandler { savePath: string, artifactName?: string ): Promise<{ Success: boolean; FilePath?: string; Error?: string }> { + // Tracked outside the try so a mid-download failure can report how far it got + // (distinguishes a stalled connection from a never-started one). + let downloadedBytes = 0 try { const downloadInfo = await this.createArtifactDownloadUrl(workspaceId, jobId, artifactId) if (!downloadInfo) { - return { Success: false, Error: 'Failed to get download URL' } + const msg = `Failed to get download URL for artifact=${artifactId} (job=${jobId})` + this.logging.error(`ATX: downloadArtifactToPath ${msg}`) + return { Success: false, Error: msg } } - const response = await got.get(downloadInfo.s3PresignedUrl, { - headers: downloadInfo.requestHeaders || {}, - responseType: 'buffer', - timeout: { request: 30000 }, - }) - await Utils.directoryExists(savePath) const fileName = artifactName ? path.basename(artifactName) : 'artifact.zip' const filePath = path.join(savePath, fileName) - fs.writeFileSync(filePath, Buffer.from(response.body)) + // Stream the artifact straight to disk instead of buffering the entire file in memory. + // Large artifacts (e.g. Transformation_Report.html) previously failed here: the old + // `timeout: { request: 30000 }` caps the WHOLE request — including receiving the full + // body — so a large-but-healthy download would abort with a TimeoutError, which was + // then swallowed and surfaced to the user as a generic "could not download" error. + // `responseType: 'buffer'` also held the full file (plus a copy) in memory. Streaming + // with inactivity-based timeouts (time-to-headers + socket idle) removes the size + // ceiling while still failing fast on a genuinely stalled connection. + this.logging.log(`ATX: downloadArtifactToPath streaming artifact=${artifactId} to ${filePath}`) + const downloadStream = got.stream(downloadInfo.s3PresignedUrl, { + headers: downloadInfo.requestHeaders || {}, + timeout: { response: 30000, socket: 60000 }, + }) + + downloadStream.on('downloadProgress', ({ transferred }) => { + downloadedBytes = transferred + }) + + await pipeline(downloadStream, fs.createWriteStream(filePath)) + + this.logging.log(`ATX: downloadArtifactToPath completed artifact=${artifactId}, bytes=${downloadedBytes}`) return { Success: true, FilePath: savePath } } catch (error) { - return { Success: false, Error: String(error) } + // Log every failure with enough context to diagnose without a repro: the got error + // `code` (ETIMEDOUT/ECONNRESET/...), the timeout phase for a TimeoutError, the HTTP + // status for an HTTPError, bytes received before the failure, and the full stack. + const err = error as any + const diagnostics: string[] = [] + if (err?.code) diagnostics.push(`code=${err.code}`) + if (err?.event) diagnostics.push(`timeoutPhase=${err.event}`) + if (err?.response?.statusCode) diagnostics.push(`httpStatus=${err.response.statusCode}`) + const suffix = diagnostics.length ? ` [${diagnostics.join(', ')}]` : '' + this.logging.error( + `ATX: downloadArtifactToPath failed artifact=${artifactId} (job=${jobId}) after ${downloadedBytes} bytes: ${String(err?.stack ?? error)}${suffix}` + ) + return { Success: false, Error: `${String(err?.message ?? error)}${suffix}` } } } diff --git a/server/aws-lsp-codewhisperer/src/language-server/netTransform/tests/atxTransformHandler.test.ts b/server/aws-lsp-codewhisperer/src/language-server/netTransform/tests/atxTransformHandler.test.ts index 76dfc8bd49..ed74c92118 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/netTransform/tests/atxTransformHandler.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/netTransform/tests/atxTransformHandler.test.ts @@ -3,6 +3,8 @@ import * as sinon from 'sinon' import * as fs from 'fs' import * as path from 'path' import * as os from 'os' +import { Readable } from 'stream' +import got from 'got' import { ATXTransformHandler } from '../atxTransformHandler' import { workspaceFolderName } from '../utils' import { AtxTokenServiceManager } from '../../../shared/amazonQServiceManager/AtxTokenServiceManager' @@ -38,6 +40,82 @@ describe('ATXTransformHandler - Chat APIs', () => { sinon.restore() }) + // The beam artifact cache had no tests at all, and the LRU-victim case below is exactly the bug + // the cap-correction commit fixed — nothing guarded the regression. + describe('downloadJsonArtifact cache', () => { + let urlStub: sinon.SinonStub + + beforeEach(() => { + urlStub = sinon.stub(handler as any, 'createArtifactDownloadUrl') + }) + + it('caches a NEGATIVE (not JSON) so the artifact is never re-fetched', async () => { + // Negatives were 1230 of the 1980 wasted fetches — the bulk of the win — and "these + // bytes are not JSON" is a permanent property, so it must not expire. + ;(handler as any).beamJsonArtifactCache.set('a1', { + value: null, + expiresAt: Number.MAX_SAFE_INTEGER, + }) + + const result = await (handler as any).downloadJsonArtifact('ws', 'job', 'a1') + + expect(result).to.equal(null) + expect(urlStub.called).to.be.false + }) + + it('serves a fresh POSITIVE from cache without re-fetching', async () => { + ;(handler as any).beamJsonArtifactCache.set('a2', { + value: { repos: ['alice'] }, + expiresAt: Date.now() + 60_000, + }) + + const result = await (handler as any).downloadJsonArtifact('ws', 'job', 'a2') + + expect(result).to.deep.equal({ repos: ['alice'] }) + expect(urlStub.called).to.be.false + }) + + it('RE-FETCHES an expired positive — a rewritten beam-map must not be pinned', async () => { + // Why positives carry a TTL: web-orc re-writes the beam-map, and if a rewrite reuses the + // artifactId a permanent cache would hide every repo beamed after the first one until + // the LSP restarted. + ;(handler as any).beamJsonArtifactCache.set('a3', { + value: { repos: ['alice'] }, + expiresAt: Date.now() - 1, + }) + urlStub.resolves(null) // no presigned URL — proves only that the fetch was ATTEMPTED + + const result = await (handler as any).downloadJsonArtifact('ws', 'job', 'a3') + + expect(urlStub.calledOnce).to.be.true + expect(result).to.equal(null) + }) + + it('does NOT cache a missing presigned URL — that is transient', async () => { + urlStub.resolves(null) + + await (handler as any).downloadJsonArtifact('ws', 'job', 'a4') + await (handler as any).downloadJsonArtifact('ws', 'job', 'a4') + + expect(urlStub.calledTwice).to.be.true + expect((handler as any).beamJsonArtifactCache.has('a4')).to.be.false + }) + + it('evicts the least-recently-USED entry, not the oldest inserted', async () => { + // Guards the cap-correction commit: a hit must refresh recency. Without the + // delete+re-insert on read, Map insertion order makes this evict 'first' — the entry + // just used — which is how an under-sized cap collapsed the hit rate to zero. + const cache = (handler as any).beamJsonArtifactCache + cache.set('first', { value: null, expiresAt: Number.MAX_SAFE_INTEGER }) + cache.set('second', { value: null, expiresAt: Number.MAX_SAFE_INTEGER }) + + // Touch 'first' so it becomes most-recently-used. + await (handler as any).downloadJsonArtifact('ws', 'job', 'first') + + expect(Array.from(cache.keys())).to.deep.equal(['second', 'first']) + }) + }) + describe('sendMessage', () => { it('should send message and return response without polling', async () => { const mockResponse = { @@ -51,7 +129,17 @@ describe('ATXTransformHandler - Chat APIs', () => { skipPolling: true, }) - expect(result).to.deep.equal({ success: true, data: mockResponse }) + // skipPolling must return the SAME shape as the polling path, not the raw send result. + // The IDE reads data.sentMessage.messageId to seed its seen-set so the 3s chat poll does + // not render the server's copy as a second bubble; returning `mockResponse` verbatim + // left that unreadable and every beamed message appeared twice. + expect(result).to.deep.equal({ + success: true, + data: { sentMessage: mockResponse.message, response: null }, + }) + // Assert the consumed path explicitly — deep.equal above would still pass if the field + // were renamed on both sides, but the IDE reads exactly this. + expect(result.data.sentMessage.messageId).to.equal('msg-123') expect(sendStub.calledOnce).to.be.true }) @@ -3220,7 +3308,60 @@ describe('ATXTransformHandler - upload flows, polling, and small wrappers', () = const result = await handler.downloadArtifactToPath('ws-1', 'job-1', 'art-1', 'C:/save') expect(result.Success).to.be.false - expect(result.Error).to.equal('Failed to get download URL') + expect(result.Error).to.contain('Failed to get download URL') + }) + + it('should stream a large artifact to disk without buffering the whole file', async () => { + const saveDir = fs.mkdtempSync(path.join(os.tmpdir(), 'atx-dl-')) + try { + sinon.stub(handler, 'createArtifactDownloadUrl').resolves({ + s3PresignedUrl: 'https://s3.example.com/report', + requestHeaders: {}, + } as any) + + // Simulate a large payload delivered as a stream (the fix path). The old + // implementation buffered this in memory under a 30s whole-request timeout. + const payload = Buffer.alloc(5 * 1024 * 1024, 'a') + sinon.stub(got, 'stream').returns(Readable.from([payload]) as any) + + const result = await handler.downloadArtifactToPath( + 'ws-1', + 'job-1', + 'art-1', + saveDir, + 'Transformation_Report.html' + ) + + expect(result.Success).to.be.true + const written = fs.readFileSync(path.join(saveDir, 'Transformation_Report.html')) + expect(written.length).to.equal(payload.length) + } finally { + fs.rmSync(saveDir, { recursive: true, force: true }) + } + }) + + it('should return Success=false with the error when the stream fails mid-download', async () => { + const saveDir = fs.mkdtempSync(path.join(os.tmpdir(), 'atx-dl-')) + try { + sinon.stub(handler, 'createArtifactDownloadUrl').resolves({ + s3PresignedUrl: 'https://s3.example.com/report', + requestHeaders: {}, + } as any) + + const failing = new Readable({ + read() { + this.destroy(new Error('socket hang up')) + }, + }) + sinon.stub(got, 'stream').returns(failing as any) + + const result = await handler.downloadArtifactToPath('ws-1', 'job-1', 'art-1', saveDir, 'artifact.zip') + + expect(result.Success).to.be.false + expect(result.Error).to.contain('socket hang up') + } finally { + fs.rmSync(saveDir, { recursive: true, force: true }) + } }) }) @@ -4432,3 +4573,59 @@ describe('ATXTransformHandler - Beam to IDE', () => { }) }) }) + +describe('ATXTransformHandler - isRepoLbvOpen: repo node terminal with no LBV child', () => { + // Both directions are asserted deliberately. The gate here reads correctly either way, so a + // one-sided test cannot tell a working gate from an unreachable one: removing the branch and + // inverting it produce different failures, and only covering both catches both. + let handler: ATXTransformHandler + + const planWith = (repoStatus: string, lbvChild?: { StepName: string; Status: string }) => ({ + StepName: 'root', + Status: 'IN_PROGRESS', + Children: [ + { + StepName: 'alice (beamed)', + Status: repoStatus, + Children: lbvChild ? [lbvChild] : [], + }, + ], + }) + + const isOpen = (plan: any) => (handler as any).isRepoLbvOpen(plan, 'alice', 'job-1') as boolean + + beforeEach(() => { + handler = new ATXTransformHandler( + sinon.createStubInstance(AtxTokenServiceManager) as any, + {} as Workspace, + { log: sinon.stub(), error: sinon.stub() } as any, + {} as Runtime + ) + }) + + afterEach(() => sinon.restore()) + + it('CLOSED when the repo node is terminal and has no LBV child — a reaped beam must drop off', () => { + // Fails if the branch is removed: without it this returns the default OPEN and the repo + // sits in the Transferred list forever with a disabled Load button. + expect(isOpen(planWith('STOPPED'))).to.be.false + }) + + it('CLOSED on a cancelled repo node with no LBV child', () => { + expect(isOpen(planWith('CANCELLED'))).to.be.false + }) + + it('OPEN when the repo node is still live and has no LBV child — a fresh transfer must stay', () => { + // Fails if the branch fires on the wrong side: over-gating here hides every repo whose + // sub-agent has not yet created its HITL, which is the normal state right after a beam. + expect(isOpen(planWith('IN_PROGRESS'))).to.be.true + }) + + it('still defers to the LBV child when one exists, terminal repo node notwithstanding', () => { + // The new branch must not shadow the original signal: with an LBV child present its status + // decides, so a repo whose build is still running stays visible even if the parent node has + // been marked terminal early. + const plan = planWith('SUCCEEDED', { StepName: 'Local Build Verification', Status: 'IN_PROGRESS' }) + expect(isOpen(plan)).to.be.true + }) +})