From b0018f849e2e8d96b66f9c732a95597a5b6c1c1e Mon Sep 17 00:00:00 2001 From: invictus <149003065+ashishrp-aws@users.noreply.github.com> Date: Tue, 11 Aug 2026 16:07:22 -0700 Subject: [PATCH 1/4] fix(amazonq): pass notification feature to the Q service manager (#2796) The access-blocked notification added in #2794 never reached the client. AmazonQServiceServerFactory destructures the features it forwards to the service manager, and notification was not among them, so features.notification was always undefined, the guard in serviceFactory never passed, onAccessBlocked was never assigned, and the notifier could not run. notification is optional on QServiceManagerFeatures so that existing constructions and test fixtures keep compiling. That is also why omitting it here did not fail the build -- it silently disabled client-facing reporting instead. Noted at the call site so the next person adding a feature there does not repeat it. Also set a stable id on the notification. Clients need to recognise it without inspecting its text: the message is the service's own copy and is expected to change, and FEATURE_NOT_SUPPORTED is reused across several RTS gates so the reason alone does not identify this one. Both IDE clients already prefer the id when present and fall back to matching the title only because the released server does not send one yet. Verified: tsc clean, prettier clean, 6/6 notifier tests pass, and a server bundle built from this branch contains the wiring where a bundle from the previous head did not. --- server/aws-lsp-codewhisperer/src/shared/amazonQServer.ts | 7 ++++++- .../src/shared/qDevAccessBlockedNotifier.test.ts | 4 +++- .../src/shared/qDevAccessBlockedNotifier.ts | 7 +++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/server/aws-lsp-codewhisperer/src/shared/amazonQServer.ts b/server/aws-lsp-codewhisperer/src/shared/amazonQServer.ts index e9c83b0a95..b386302e71 100644 --- a/server/aws-lsp-codewhisperer/src/shared/amazonQServer.ts +++ b/server/aws-lsp-codewhisperer/src/shared/amazonQServer.ts @@ -19,7 +19,7 @@ const LOGGING_PREFIX = '[AMAZON Q SERVER]: ' export const AmazonQServiceServerFactory = (serviceManager: (features: QServiceManagerFeatures) => AmazonQBaseServiceManager): Server => - ({ credentialsProvider, lsp, workspace, logging, runtime, sdkInitializator }) => { + ({ credentialsProvider, lsp, workspace, logging, runtime, sdkInitializator, notification }) => { let amazonQServiceManager: AmazonQBaseServiceManager const log = (message: string) => { @@ -39,6 +39,11 @@ export const AmazonQServiceServerFactory = logging, runtime, sdkInitializator, + // Required for the service manager to be able to surface anything to the client. It is + // optional on QServiceManagerFeatures so that existing constructions (including test + // fixtures) keep compiling, which means omitting it here does not fail the build -- it + // just silently disables client-facing reporting. + notification, }) /* diff --git a/server/aws-lsp-codewhisperer/src/shared/qDevAccessBlockedNotifier.test.ts b/server/aws-lsp-codewhisperer/src/shared/qDevAccessBlockedNotifier.test.ts index 1914680154..858fe3d8cc 100644 --- a/server/aws-lsp-codewhisperer/src/shared/qDevAccessBlockedNotifier.test.ts +++ b/server/aws-lsp-codewhisperer/src/shared/qDevAccessBlockedNotifier.test.ts @@ -7,7 +7,7 @@ import { MessageType } from '@aws/language-server-runtimes/protocol' import { Logging, Notification } from '@aws/language-server-runtimes/server-interface' import * as assert from 'assert' import * as sinon from 'sinon' -import { createQDevAccessBlockedNotifier } from './qDevAccessBlockedNotifier' +import { createQDevAccessBlockedNotifier, Q_DEV_ACCESS_BLOCKED_NOTIFICATION_ID } from './qDevAccessBlockedNotifier' describe('createQDevAccessBlockedNotifier', function () { let showNotification: sinon.SinonStub @@ -40,6 +40,8 @@ describe('createQDevAccessBlockedNotifier', function () { const params = showNotification.firstCall.args[0] assert.strictEqual(params.type, MessageType.Error) assert.strictEqual(params.content.text, serviceMessage) + // Clients key off the id rather than the message text, which is service-owned copy. + assert.strictEqual(params.id, Q_DEV_ACCESS_BLOCKED_NOTIFICATION_ID) }) it('notifies at most once, since every request from a blocked identity fails', function () { diff --git a/server/aws-lsp-codewhisperer/src/shared/qDevAccessBlockedNotifier.ts b/server/aws-lsp-codewhisperer/src/shared/qDevAccessBlockedNotifier.ts index 7f61207fb5..6da7b4b38a 100644 --- a/server/aws-lsp-codewhisperer/src/shared/qDevAccessBlockedNotifier.ts +++ b/server/aws-lsp-codewhisperer/src/shared/qDevAccessBlockedNotifier.ts @@ -16,6 +16,12 @@ const FALLBACK_TEXT = 'Amazon Q Developer is not available for this account.' const TITLE = 'Amazon Q Developer' +/** + * Stable identifier so clients can recognise this notification without inspecting its text. Clients + * must not match on the message: it is the service's own copy and is expected to change. + */ +export const Q_DEV_ACCESS_BLOCKED_NOTIFICATION_ID = 'qDevPluginAccessBlocked' + /** * Builds the reaction to RTS blocking Q Developer plugin access for the current identity, for use as * {@link CodeWhispererServiceToken.onAccessBlocked}. @@ -56,6 +62,7 @@ export function createQDevAccessBlockedNotifier( try { logging.warn(`Q Developer plugin access is blocked for this identity: ${text}`) notification.showNotification({ + id: Q_DEV_ACCESS_BLOCKED_NOTIFICATION_ID, type: MessageType.Error, content: { title: TITLE, From 13dd9d09d1ff3ee0196b168dd37331040231d27f Mon Sep 17 00:00:00 2001 From: invictus <149003065+ashishrp-aws@users.noreply.github.com> Date: Wed, 12 Aug 2026 14:11:41 -0700 Subject: [PATCH 2/4] fix(amazonq): declare serverInfo so notifications reach the client (#2797) * fix(amazonq): declare serverInfo so notifications reach the client The access-blocked notification still never reached the client after #2796. The runtime only constructs a notification router for servers that declare serverInfo: if (initializeResult?.serverInfo) { this.notificationRouter = new RouterByServerName(initializeResult.serverInfo.name, ...) } AmazonQServiceServer returned only capabilities and awsServerCapabilities, so the router was never built and notification.showNotification() logged "Notifications are not supported: serverInfo is not defined" and dropped the notification. Observed in VS Code: the block was detected and logged, then silently discarded. This is the last piece. With #2794 (detect), #2796 (wire) and this change (deliver), a blocked identity produces a notification the client can act on. Added a regression test, because the failure mode is silent: nothing throws and only a debug line marks the loss. The test asserts the exact name, which is deliberate -- the name is encoded into the id of every notification the client echoes back, so renaming it strands followups for notifications already on screen. Note: amazonQServer.test.ts has one pre-existing failure on this branch, "hooks onUpdateConfiguration handler to LSP server", present before this change (6 passing/1 failing before, 7 passing/1 failing after). Left alone as unrelated. * fix(amazonq): observe access-blocked on the streaming client too The observer added in #2794 was only on the token client. Chat runs through the streaming client, so the one surface where a blocked identity actually shows up to the user was the one place nothing was watching. Detection happened to work anyway because the gate denies every operation and the A/B config fetch goes through the token client moments after credentials arrive -- but that is incidental, not a guarantee. Mirrors the token client exactly: middleware on the outermost initialize step so it fires once per operation after retries are exhausted, the observer is called inside its own try/catch, and the error is always rethrown so callers behave as before. The notifier is now created once per service generation and shared by both clients rather than created per client. The notifier dedupes per instance, so sharing is what keeps a blocked identity to a single notification no matter which client sees it first. It is cleared by resetCodewhispererService, so signing out and back in with another blocked identity notifies again instead of being suppressed. Scoped to StreamingClientServiceToken. The IAM variant serves a different surface and the gate only denies Builder ID, which is bearer-token only. Pre-existing failures on this branch, unchanged by this commit: utils.test.ts 11 failing (89 passing) and amazonQServer.test.ts 1 failing, both identical before and after. --- .../src/shared/amazonQServer.test.ts | 19 ++++++++++ .../src/shared/amazonQServer.ts | 8 +++++ .../AmazonQTokenServiceManager.test.ts | 9 +++++ .../AmazonQTokenServiceManager.ts | 30 ++++++++++++++-- .../src/shared/streamingClientService.ts | 35 ++++++++++++++++++- 5 files changed, 97 insertions(+), 4 deletions(-) diff --git a/server/aws-lsp-codewhisperer/src/shared/amazonQServer.test.ts b/server/aws-lsp-codewhisperer/src/shared/amazonQServer.test.ts index 7d6e9258fb..d4874d3690 100644 --- a/server/aws-lsp-codewhisperer/src/shared/amazonQServer.test.ts +++ b/server/aws-lsp-codewhisperer/src/shared/amazonQServer.test.ts @@ -6,6 +6,7 @@ import { CancellationToken, CredentialsType, InitializeParams, + PartialInitializeResult, Server, UpdateConfigurationParams, } from '@aws/language-server-runtimes/server-interface' @@ -43,6 +44,24 @@ describe('AmazonQServiceServer', () => { sinon.assert.calledOnce(initBaseTestServiceManagerSpy) }) + it('declares serverInfo so the runtime can deliver notifications to the client', async () => { + server(features) + + // Invoke the registered initializer directly: doSendInitializeRequest returns void, so the + // result is only reachable through the handler the server registered. + const initializer = features.lsp.addInitializer.args[0]?.[0] + const result = (await initializer({} as InitializeParams, {} as CancellationToken)) as PartialInitializeResult + + // The runtime only builds a notification router for servers that declare serverInfo, and + // notification.showNotification() is a silent no-op without one. Dropping this makes every + // server-initiated notification disappear with nothing but a debug line to show for it. + // + // The name is asserted exactly because it is not cosmetic: it is encoded into the id of each + // notification the client sends back, so renaming it strands followups for notifications + // already on screen. + expect(result.serverInfo?.name).to.equal('AWS Language Server for Amazon Q Developer') + }) + it('hooks handleDidChangeConfiguration to didChangeConfiguration and onInitialized handlers', async () => { const handleDidChangeConfigurationSpy = sinon.spy( BaseAmazonQServiceManager.prototype, diff --git a/server/aws-lsp-codewhisperer/src/shared/amazonQServer.ts b/server/aws-lsp-codewhisperer/src/shared/amazonQServer.ts index b386302e71..8af68702a4 100644 --- a/server/aws-lsp-codewhisperer/src/shared/amazonQServer.ts +++ b/server/aws-lsp-codewhisperer/src/shared/amazonQServer.ts @@ -63,6 +63,14 @@ export const AmazonQServiceServerFactory = return { capabilities: {}, awsServerCapabilities: {}, + // Required for anything to reach the client through the notification feature. The + // runtime only builds a notification router when a server declares serverInfo, and + // showNotification is a silent no-op without one. Not exposed to clients; it is used + // internally to route notification followups back to the originating server, so the + // name must stay stable. + serverInfo: { + name: 'AWS Language Server for Amazon Q Developer', + }, } }) diff --git a/server/aws-lsp-codewhisperer/src/shared/amazonQServiceManager/AmazonQTokenServiceManager.test.ts b/server/aws-lsp-codewhisperer/src/shared/amazonQServiceManager/AmazonQTokenServiceManager.test.ts index ace2ae9943..c93552a7d7 100644 --- a/server/aws-lsp-codewhisperer/src/shared/amazonQServiceManager/AmazonQTokenServiceManager.test.ts +++ b/server/aws-lsp-codewhisperer/src/shared/amazonQServiceManager/AmazonQTokenServiceManager.test.ts @@ -238,6 +238,15 @@ describe('AmazonQTokenServiceManager', () => { assert(codewhispererServiceStub.generateSuggestions.calledOnce) }) + it('gives the streaming client an access-blocked observer', async () => { + const streamingClient = amazonQTokenServiceManager.getStreamingClient() + + // Chat runs through the streaming client, so without this it is the one surface where a + // blocked identity shows up and the one place nothing observes it. The token client is + // a stub in this harness, so only the streaming side is asserted here. + assert.strictEqual(typeof streamingClient.onAccessBlocked, 'function') + }) + it('should initialize service with region set by client', async () => { features.setClientParams({ processId: 0, diff --git a/server/aws-lsp-codewhisperer/src/shared/amazonQServiceManager/AmazonQTokenServiceManager.ts b/server/aws-lsp-codewhisperer/src/shared/amazonQServiceManager/AmazonQTokenServiceManager.ts index 47b43d0d81..608d4ae2ed 100644 --- a/server/aws-lsp-codewhisperer/src/shared/amazonQServiceManager/AmazonQTokenServiceManager.ts +++ b/server/aws-lsp-codewhisperer/src/shared/amazonQServiceManager/AmazonQTokenServiceManager.ts @@ -76,6 +76,7 @@ export class AmazonQTokenServiceManager extends BaseAmazonQServiceManager< private region?: string private endpoint?: string private regionChangeListeners: Array<(region: string) => void> = [] + private cachedQDevAccessBlockedNotifier?: (error: unknown) => void /** * Internal state of Service connection, based on status of bearer token and Amazon Q Developer profile selection. @@ -525,12 +526,34 @@ export class AmazonQTokenServiceManager extends BaseAmazonQServiceManager< return this.cachedStreamingClient } + /** + * One notifier shared by the token and streaming clients, so a blocked identity produces a single + * notification regardless of which client observes it first (the notifier dedupes per instance). + * + * Deliberately scoped to the current service generation and cleared by + * {@link resetCodewhispererService}: signing out and back in with another blocked identity must + * notify again, which a manager-lifetime notifier would suppress. + */ + private getQDevAccessBlockedNotifier(): ((error: unknown) => void) | undefined { + if (!this.features.notification) { + return undefined + } + + this.cachedQDevAccessBlockedNotifier ??= createQDevAccessBlockedNotifier( + this.features.notification, + this.features.logging + ) + + return this.cachedQDevAccessBlockedNotifier + } + private resetCodewhispererService() { this.logging.log('Resetting Q-only services') this.cachedCodewhispererService?.abortInflightRequests() this.cachedCodewhispererService = undefined this.cachedStreamingClient?.abortInflightRequests() this.cachedStreamingClient = undefined + this.cachedQDevAccessBlockedNotifier = undefined this.activeIdcProfile = undefined this.region = undefined this.endpoint = undefined @@ -597,9 +620,7 @@ export class AmazonQTokenServiceManager extends BaseAmazonQServiceManager< service.customizationArn = this.configurationCache.getProperty('customizationArn') service.profileArn = this.activeIdcProfile?.arn - if (this.features.notification) { - service.onAccessBlocked = createQDevAccessBlockedNotifier(this.features.notification, this.features.logging) - } + service.onAccessBlocked = this.getQDevAccessBlockedNotifier() service.shareCodeWhispererContentWithAWS = this.configurationCache.getProperty( 'shareCodeWhispererContentWithAWS' ) @@ -638,6 +659,9 @@ export class AmazonQTokenServiceManager extends BaseAmazonQServiceManager< endpoint, this.getCustomUserAgent() ) + // Chat runs through the streaming client, so it needs the same observer as the token client -- + // otherwise a blocked identity goes unnoticed unless some other client happens to be called. + streamingClient.onAccessBlocked = this.getQDevAccessBlockedNotifier() streamingClient.profileArn = this.activeIdcProfile?.arn streamingClient.shareCodeWhispererContentWithAWS = this.configurationCache.getProperty( 'shareCodeWhispererContentWithAWS' diff --git a/server/aws-lsp-codewhisperer/src/shared/streamingClientService.ts b/server/aws-lsp-codewhisperer/src/shared/streamingClientService.ts index 373c2b54c4..d14c24fecf 100644 --- a/server/aws-lsp-codewhisperer/src/shared/streamingClientService.ts +++ b/server/aws-lsp-codewhisperer/src/shared/streamingClientService.ts @@ -18,7 +18,7 @@ import { Logging, IamCredentials, } from '@aws/language-server-runtimes/server-interface' -import { getBearerTokenFromProvider, isUsageLimitError } from './utils' +import { getBearerTokenFromProvider, isUsageLimitError, isQDevPluginAccessBlockedError } from './utils' import { CLIENT_TIMEOUT_MS, MAX_REQUEST_ATTEMPTS } from '../language-server/agenticChat/constants/constants' import { AmazonQUsageLimitError } from './amazonQServiceManager/errors' @@ -72,6 +72,12 @@ export abstract class StreamingClientServiceBase { } export class StreamingClientServiceToken extends StreamingClientServiceBase { + /** + * Observes Q Developer plugin access-blocked rejections. Assigned by the service manager after + * construction, so the middleware below reads it at call time rather than capturing it. + */ + public onAccessBlocked?: (error: unknown) => void + client: CodeWhispererStreaming public profileArn?: string private retryClassifier: QRetryClassifier @@ -132,6 +138,33 @@ export class StreamingClientServiceToken extends StreamingClientServiceBase { step: 'build', } ) + + // Observe access-blocked rejections without altering behaviour: the error is always rethrown + // so callers see it exactly as before. Registered on the outermost (initialize) step so it + // fires once per operation, after the SDK's retries are exhausted, rather than once per + // attempt. Chat runs through this client, so without it a blocked identity goes unnoticed + // unless some other client happens to be called. + this.client.middlewareStack.add( + next => async args => { + try { + return await next(args) + } catch (e) { + if (isQDevPluginAccessBlockedError(e)) { + try { + this.onAccessBlocked?.(e) + } catch (observerError) { + logging.debug( + `onAccessBlocked observer threw, ignoring: ${(observerError as Error)?.message}` + ) + } + } + throw e + } + }, + { + step: 'initialize', + } + ) } public async sendMessage( From f5296736c6174c80b290c42eba7d3c11b977e033 Mon Sep 17 00:00:00 2001 From: invictus <149003065+ashishrp-aws@users.noreply.github.com> Date: Wed, 12 Aug 2026 16:38:34 -0700 Subject: [PATCH 3/4] fix(amazonq): give the IAM and token servers distinct serverInfo names (#2799) The serverInfo added in #2797 used one hardcoded name, but AmazonQServiceServerFactory is instantiated twice -- AmazonQServiceServerIAM and AmazonQServiceServerToken -- and runtimes including agent-standalone register both. Two servers reporting the same name makes lspRouter reject initialize outright: Duplicate servers defined: AWS Language Server for Amazon Q Developer That fails the whole language server, not just the duplicate. Observed in VS Code as: Failed to start downloaded LSP, falling back to bundled LSP: Duplicate servers defined: AWS Language Server for Amazon Q Developer The client then silently ran its bundled server instead, so Q appeared to work while none of the access-blocked reporting existed, with only a client-side warning to show for it. serverName is now a required parameter rather than a shared constant, since a default is precisely what let two instantiations collide. The two names are exported so the uniqueness is assertable, and they must stay stable: the name is encoded into the id of every notification the client echoes back. Added a regression test on the distinctness. Verified it bites -- reintroducing the collision gives 7 passing/2 failing, the fix gives 8 passing/1 failing. No existing test registers two servers from one runtime, which is why this reached a release. Pre-existing failure on this branch, unchanged: amazonQServer.test.ts "hooks onUpdateConfiguration handler to LSP server". --- .../src/shared/amazonQServer.test.ts | 35 +++++++++++++------ .../src/shared/amazonQServer.ts | 28 ++++++++++++--- 2 files changed, 49 insertions(+), 14 deletions(-) diff --git a/server/aws-lsp-codewhisperer/src/shared/amazonQServer.test.ts b/server/aws-lsp-codewhisperer/src/shared/amazonQServer.test.ts index d4874d3690..3edcc4c097 100644 --- a/server/aws-lsp-codewhisperer/src/shared/amazonQServer.test.ts +++ b/server/aws-lsp-codewhisperer/src/shared/amazonQServer.test.ts @@ -10,9 +10,15 @@ import { Server, UpdateConfigurationParams, } from '@aws/language-server-runtimes/server-interface' -import { AmazonQServiceServerFactory } from './amazonQServer' +import { + AMAZON_Q_SERVICE_SERVER_IAM_NAME, + AMAZON_Q_SERVICE_SERVER_TOKEN_NAME, + AmazonQServiceServerFactory, +} from './amazonQServer' import { BaseAmazonQServiceManager } from './amazonQServiceManager/BaseAmazonQServiceManager' +const TEST_SERVER_NAME = 'Test Amazon Q Server' + describe('AmazonQServiceServer', () => { let features: TestFeatures let server: Server @@ -24,7 +30,7 @@ describe('AmazonQServiceServer', () => { initBaseTestServiceManagerSpy = sinon.spy(initBaseTestServiceManager) TestAmazonQServiceManager.resetInstance() - server = AmazonQServiceServerFactory(() => initBaseTestServiceManagerSpy(features)) + server = AmazonQServiceServerFactory(() => initBaseTestServiceManagerSpy(features), TEST_SERVER_NAME) }) afterEach(() => { @@ -53,13 +59,22 @@ describe('AmazonQServiceServer', () => { const result = (await initializer({} as InitializeParams, {} as CancellationToken)) as PartialInitializeResult // The runtime only builds a notification router for servers that declare serverInfo, and - // notification.showNotification() is a silent no-op without one. Dropping this makes every - // server-initiated notification disappear with nothing but a debug line to show for it. - // - // The name is asserted exactly because it is not cosmetic: it is encoded into the id of each - // notification the client sends back, so renaming it strands followups for notifications - // already on screen. - expect(result.serverInfo?.name).to.equal('AWS Language Server for Amazon Q Developer') + // notification.showNotification() is a silent no-op without one. + expect(result.serverInfo?.name).to.equal(TEST_SERVER_NAME) + }) + + it('gives the IAM and token servers distinct serverInfo names', () => { + // Regression guard. Runtimes such as agent-standalone register BOTH of these servers, and the + // runtime rejects initialize with `Duplicate servers defined` when two servers report the same + // name -- which fails the entire language server, not just the duplicate. A shared name here + // made every such runtime fall back to whatever server the client had bundled, visible only as + // a client-side warning, so Q kept working while silently running a different server. + const names = [AMAZON_Q_SERVICE_SERVER_IAM_NAME, AMAZON_Q_SERVICE_SERVER_TOKEN_NAME] + + for (const name of names) { + expect(name).to.be.a('string').and.not.empty + } + expect(new Set(names).size, `server names must be unique: ${names.join(', ')}`).to.equal(names.length) }) it('hooks handleDidChangeConfiguration to didChangeConfiguration and onInitialized handlers', async () => { @@ -130,7 +145,7 @@ describe('AmazonQServiceServer', () => { throw new Error('Service manager initialization failed') } - const errorServer = AmazonQServiceServerFactory(errorFactory) + const errorServer = AmazonQServiceServerFactory(errorFactory, TEST_SERVER_NAME) expect(() => { errorServer(features) diff --git a/server/aws-lsp-codewhisperer/src/shared/amazonQServer.ts b/server/aws-lsp-codewhisperer/src/shared/amazonQServer.ts index 8af68702a4..8ec4e19372 100644 --- a/server/aws-lsp-codewhisperer/src/shared/amazonQServer.ts +++ b/server/aws-lsp-codewhisperer/src/shared/amazonQServer.ts @@ -17,8 +17,15 @@ import { const LOGGING_PREFIX = '[AMAZON Q SERVER]: ' +/** + * @param serverName Value for the server's `serverInfo.name`. Required, and required to be unique + * across every server registered in the same runtime: the runtime rejects `initialize` outright with + * `Duplicate servers defined` if two servers report the same name, which takes down the whole language + * server rather than the offending one. This factory is instantiated more than once per runtime (IAM + * and token), so a shared constant here breaks every bundle that registers both. + */ export const AmazonQServiceServerFactory = - (serviceManager: (features: QServiceManagerFeatures) => AmazonQBaseServiceManager): Server => + (serviceManager: (features: QServiceManagerFeatures) => AmazonQBaseServiceManager, serverName: string): Server => ({ credentialsProvider, lsp, workspace, logging, runtime, sdkInitializator, notification }) => { let amazonQServiceManager: AmazonQBaseServiceManager @@ -69,7 +76,7 @@ export const AmazonQServiceServerFactory = // internally to route notification followups back to the originating server, so the // name must stay stable. serverInfo: { - name: 'AWS Language Server for Amazon Q Developer', + name: serverName, }, } }) @@ -123,5 +130,18 @@ export const AmazonQServiceServerFactory = return () => {} } -export const AmazonQServiceServerIAM = AmazonQServiceServerFactory(initBaseIAMServiceManager) -export const AmazonQServiceServerToken = AmazonQServiceServerFactory(initBaseTokenServiceManager) +// Must stay distinct from each other, and from every other server registered in the same runtime, or +// the runtime fails initialize with `Duplicate servers defined`. Must also stay stable over time: they +// are encoded into the id of every notification the client echoes back, so renaming strands followups +// for notifications already on screen. Exported so the uniqueness can be asserted in tests. +export const AMAZON_Q_SERVICE_SERVER_IAM_NAME = 'AWS Language Server for Amazon Q Developer (IAM)' +export const AMAZON_Q_SERVICE_SERVER_TOKEN_NAME = 'AWS Language Server for Amazon Q Developer (Token)' + +export const AmazonQServiceServerIAM = AmazonQServiceServerFactory( + initBaseIAMServiceManager, + AMAZON_Q_SERVICE_SERVER_IAM_NAME +) +export const AmazonQServiceServerToken = AmazonQServiceServerFactory( + initBaseTokenServiceManager, + AMAZON_Q_SERVICE_SERVER_TOKEN_NAME +) From 60a3491b8bf57d8dce581dc58146c6010b92f106 Mon Sep 17 00:00:00 2001 From: invictus <149003065+ashishrp-aws@users.noreply.github.com> Date: Thu, 13 Aug 2026 14:12:50 -0700 Subject: [PATCH 4/4] test(amazonq): address review findings on the access-blocked observer (#2801) Three review follow-ups, no behaviour change for users. Name the streaming client's middleware, matching the token client. Without a name a second registration stacks another observer rather than replacing the first, which would report the same block twice, and the middleware is anonymous in SDK stack introspection. Assert the server-name uniqueness against the real exported servers rather than the two constants. Comparing constants cannot catch the same name being passed to both factory calls, which is the mistake that actually shipped. Verified the test bites: making the names identical fails it (7 passing/2 failing vs 8/1). Add two tests for the streaming observer. They assert the wiring rather than the callback because the existing harness stubs CodeWhispererStreaming.prototype.sendMessage, which bypasses the middleware stack entirely -- a behavioural test there would pass even if the middleware did not exist. shared group: 337 passing / 45 failing, against 334 / 45 before, so the 3 new tests and no new failures. --- .../src/shared/amazonQServer.test.ts | 44 ++++++++++++++----- .../src/shared/streamingClientService.test.ts | 26 +++++++++++ .../src/shared/streamingClientService.ts | 4 ++ 3 files changed, 64 insertions(+), 10 deletions(-) diff --git a/server/aws-lsp-codewhisperer/src/shared/amazonQServer.test.ts b/server/aws-lsp-codewhisperer/src/shared/amazonQServer.test.ts index 3edcc4c097..5ad0522a0b 100644 --- a/server/aws-lsp-codewhisperer/src/shared/amazonQServer.test.ts +++ b/server/aws-lsp-codewhisperer/src/shared/amazonQServer.test.ts @@ -14,6 +14,8 @@ import { AMAZON_Q_SERVICE_SERVER_IAM_NAME, AMAZON_Q_SERVICE_SERVER_TOKEN_NAME, AmazonQServiceServerFactory, + AmazonQServiceServerIAM, + AmazonQServiceServerToken, } from './amazonQServer' import { BaseAmazonQServiceManager } from './amazonQServiceManager/BaseAmazonQServiceManager' @@ -63,17 +65,39 @@ describe('AmazonQServiceServer', () => { expect(result.serverInfo?.name).to.equal(TEST_SERVER_NAME) }) - it('gives the IAM and token servers distinct serverInfo names', () => { - // Regression guard. Runtimes such as agent-standalone register BOTH of these servers, and the - // runtime rejects initialize with `Duplicate servers defined` when two servers report the same - // name -- which fails the entire language server, not just the duplicate. A shared name here - // made every such runtime fall back to whatever server the client had bundled, visible only as - // a client-side warning, so Q kept working while silently running a different server. - const names = [AMAZON_Q_SERVICE_SERVER_IAM_NAME, AMAZON_Q_SERVICE_SERVER_TOKEN_NAME] - - for (const name of names) { - expect(name).to.be.a('string').and.not.empty + it('gives the IAM and token servers distinct serverInfo names', async () => { + // Regression guard, asserted against the real exported servers rather than the constants, so + // it also catches the same name being passed to both factory calls. + // + // Runtimes such as agent-standalone register BOTH of these servers, and the runtime rejects + // initialize with `Duplicate servers defined` when two servers report the same name -- which + // fails the entire language server, not just the duplicate. A shared name here made every such + // runtime fall back to whatever server the client had bundled, visible only as a client-side + // warning, so Q kept working while silently running a different server. + const names: (string | undefined)[] = [] + + for (const qServer of [AmazonQServiceServerIAM, AmazonQServiceServerToken]) { + const serverFeatures = new TestFeatures() + try { + // The service managers refuse to initialize before the LSP connection has, so the + // client params have to be in place before the initializer runs. + serverFeatures.setClientParams({} as InitializeParams) + qServer(serverFeatures) + + const initializer = serverFeatures.lsp.addInitializer.args[0]?.[0] + const result = (await initializer( + {} as InitializeParams, + {} as CancellationToken + )) as PartialInitializeResult + + names.push(result.serverInfo?.name) + } finally { + serverFeatures.dispose() + TestAmazonQServiceManager.resetInstance() + } } + + expect(names).to.deep.equal([AMAZON_Q_SERVICE_SERVER_IAM_NAME, AMAZON_Q_SERVICE_SERVER_TOKEN_NAME]) expect(new Set(names).size, `server names must be unique: ${names.join(', ')}`).to.equal(names.length) }) diff --git a/server/aws-lsp-codewhisperer/src/shared/streamingClientService.test.ts b/server/aws-lsp-codewhisperer/src/shared/streamingClientService.test.ts index 5b1f08d851..382052cce7 100644 --- a/server/aws-lsp-codewhisperer/src/shared/streamingClientService.test.ts +++ b/server/aws-lsp-codewhisperer/src/shared/streamingClientService.test.ts @@ -142,6 +142,32 @@ describe('StreamingClientServiceToken', () => { expect(streamingClientServiceDefault['shareCodeWhispererContentWithAWS']).to.be.undefined }) + describe('access-blocked observer', () => { + it('registers exactly one named middleware on the initialize step', () => { + // Guards the wiring rather than the callback: the harness stubs + // CodeWhispererStreaming.prototype.sendMessage, which bypasses the middleware stack, so a + // behavioural test here would pass without the middleware existing at all. + // + // The name matters. Without it a second registration stacks another observer instead of + // replacing the first, which would report the same block twice. + const registered = streamingClientService.client.middlewareStack + .identify() + .filter(entry => entry.includes('detectQDevPluginAccessBlocked')) + + expect(registered).to.have.lengthOf(1) + expect(registered[0]).to.contain('initialize') + }) + + it('exposes a settable observer for the service manager to assign', () => { + // Assigned after construction, so the middleware has to read it at call time. If this + // became readonly or were dropped, chat-time blocks would go unobserved. + const observer = () => {} + streamingClientService.onAccessBlocked = observer + + expect(streamingClientService.onAccessBlocked).to.equal(observer) + }) + }) + describe('generateAssistantResponse', () => { const MOCKED_GENERATE_RESPONSE_REQUEST = { conversationState: { diff --git a/server/aws-lsp-codewhisperer/src/shared/streamingClientService.ts b/server/aws-lsp-codewhisperer/src/shared/streamingClientService.ts index d14c24fecf..dbb6aa98c3 100644 --- a/server/aws-lsp-codewhisperer/src/shared/streamingClientService.ts +++ b/server/aws-lsp-codewhisperer/src/shared/streamingClientService.ts @@ -163,6 +163,10 @@ export class StreamingClientServiceToken extends StreamingClientServiceBase { }, { step: 'initialize', + // Named so a second registration replaces this one rather than stacking another + // observer, and so the middleware is identifiable in SDK stack introspection. Matches + // the token client's registration. + name: 'detectQDevPluginAccessBlocked', } ) }