From 9763dd46f6302913053e4f62e23c9f264adf79dd Mon Sep 17 00:00:00 2001 From: Blake Lazarine Date: Thu, 31 Jul 2025 15:39:10 -0700 Subject: [PATCH 1/4] feat(amazonq): enable displayFindings tool --- packages/amazonq/src/lsp/chat/messages.ts | 19 +++++++++++--- packages/amazonq/src/lsp/client.ts | 1 + .../src/codewhisperer/models/constants.ts | 6 ++++- .../service/diagnosticsProvider.ts | 23 +++++++++-------- .../service/securityIssueProvider.ts | 25 +++++++++++++++++++ 5 files changed, 60 insertions(+), 14 deletions(-) diff --git a/packages/amazonq/src/lsp/chat/messages.ts b/packages/amazonq/src/lsp/chat/messages.ts index a95b99b442c..16965e2f41f 100644 --- a/packages/amazonq/src/lsp/chat/messages.ts +++ b/packages/amazonq/src/lsp/chat/messages.ts @@ -732,7 +732,11 @@ async function handlePartialResult( // This is to filter out the message containing findings from CodeReview tool to update CodeIssues panel decryptedMessage.additionalMessages = decryptedMessage.additionalMessages?.filter( (message) => - !(message.messageId !== undefined && message.messageId.endsWith(CodeWhispererConstants.findingsSuffix)) + !( + message.messageId !== undefined && + (message.messageId.endsWith(CodeWhispererConstants.codeReviewFindingsSuffix) || + message.messageId.endsWith(CodeWhispererConstants.displayFindingsSuffix)) + ) ) if (decryptedMessage.body !== undefined) { @@ -784,7 +788,11 @@ async function handleSecurityFindings( } for (let i = decryptedMessage.additionalMessages.length - 1; i >= 0; i--) { const message = decryptedMessage.additionalMessages[i] - if (message.messageId !== undefined && message.messageId.endsWith(CodeWhispererConstants.findingsSuffix)) { + if ( + message.messageId !== undefined && + (message.messageId.endsWith(CodeWhispererConstants.codeReviewFindingsSuffix) || + message.messageId.endsWith(CodeWhispererConstants.displayFindingsSuffix)) + ) { if (message.body !== undefined) { try { const aggregatedCodeScanIssues: AggregatedCodeScanIssue[] = JSON.parse(message.body) @@ -803,7 +811,12 @@ async function handleSecurityFindings( issue.visible = !isIssueTitleIgnored && !isSingleIssueIgnored } } - initSecurityScanRender(aggregatedCodeScanIssues, undefined, CodeAnalysisScope.PROJECT) + initSecurityScanRender( + aggregatedCodeScanIssues, + undefined, + CodeAnalysisScope.AGENTIC, + message.messageId.endsWith(CodeWhispererConstants.codeReviewFindingsSuffix) + ) SecurityIssueTreeViewProvider.focus() } catch (e) { languageClient.info('Failed to parse findings') diff --git a/packages/amazonq/src/lsp/client.ts b/packages/amazonq/src/lsp/client.ts index c34ae30292d..16ed5357075 100644 --- a/packages/amazonq/src/lsp/client.ts +++ b/packages/amazonq/src/lsp/client.ts @@ -182,6 +182,7 @@ export async function startLanguageServer( modelSelection: true, workspaceFilePath: vscode.workspace.workspaceFile?.fsPath, codeReviewInChat: codeReviewInChat, + displayFindings: true, }, window: { notifications: true, diff --git a/packages/core/src/codewhisperer/models/constants.ts b/packages/core/src/codewhisperer/models/constants.ts index 9e1eb2b7f94..703b21d671e 100644 --- a/packages/core/src/codewhisperer/models/constants.ts +++ b/packages/core/src/codewhisperer/models/constants.ts @@ -841,6 +841,7 @@ export enum CodeAnalysisScope { FILE_AUTO = 'FILE_AUTO', FILE_ON_DEMAND = 'FILE_ON_DEMAND', PROJECT = 'PROJECT', + AGENTIC = 'AGENTIC', } export enum TestGenerationJobStatus { @@ -907,4 +908,7 @@ export const predictionTrackerDefaultConfig = { maxSupplementalContext: 15, } -export const findingsSuffix = '_codeReviewFindings' +export const codeReviewFindingsSuffix = '_codeReviewFindings' +export const displayFindingsSuffix = '_displayFindings' + +export const displayFindingsDetectorName = 'DisplayFindings' diff --git a/packages/core/src/codewhisperer/service/diagnosticsProvider.ts b/packages/core/src/codewhisperer/service/diagnosticsProvider.ts index f181bdb146d..b7a950bba5c 100644 --- a/packages/core/src/codewhisperer/service/diagnosticsProvider.ts +++ b/packages/core/src/codewhisperer/service/diagnosticsProvider.ts @@ -26,8 +26,13 @@ export const securityScanRender: SecurityScanRender = { export function initSecurityScanRender( securityRecommendationList: AggregatedCodeScanIssue[], editor: vscode.TextEditor | undefined, - scope: CodeAnalysisScope + scope: CodeAnalysisScope, + fromQCA: boolean = true ) { + // fromQCA parameter is used to determine if the findings are coming from QCA or from displayFindings tool. + // if the incoming findings are from QCA review, then keep only existing findings from displayFindings + // if the incoming findings are not from QCA review, then keep only the existing QCA findings + securityScanRender.securityDiagnosticCollection = createSecurityDiagnosticCollection() securityScanRender.initialized = false if (scope === CodeAnalysisScope.FILE_ON_DEMAND && editor) { securityScanRender.securityDiagnosticCollection?.delete(editor.document.uri) @@ -36,22 +41,20 @@ export function initSecurityScanRender( } for (const securityRecommendation of securityRecommendationList) { updateSecurityDiagnosticCollection(securityRecommendation) - updateSecurityIssuesForProviders(securityRecommendation, scope === CodeAnalysisScope.FILE_AUTO) + updateSecurityIssuesForProviders(securityRecommendation, scope === CodeAnalysisScope.FILE_AUTO, fromQCA) } securityScanRender.initialized = true } -function updateSecurityIssuesForProviders(securityRecommendation: AggregatedCodeScanIssue, isAutoScope?: boolean) { +function updateSecurityIssuesForProviders( + securityRecommendation: AggregatedCodeScanIssue, + isAutoScope?: boolean, + fromQCA: boolean = true +) { if (isAutoScope) { SecurityIssueProvider.instance.mergeIssues(securityRecommendation) } else { - const updatedSecurityRecommendationList = [ - ...SecurityIssueProvider.instance.issues.filter( - (group) => group.filePath !== securityRecommendation.filePath - ), - securityRecommendation, - ] - SecurityIssueProvider.instance.issues = updatedSecurityRecommendationList + SecurityIssueProvider.instance.mergeIssuesDisplayFindings(securityRecommendation, fromQCA) } SecurityIssueTreeViewProvider.instance.refresh() } diff --git a/packages/core/src/codewhisperer/service/securityIssueProvider.ts b/packages/core/src/codewhisperer/service/securityIssueProvider.ts index d055cb0a7d5..01f1cd880bd 100644 --- a/packages/core/src/codewhisperer/service/securityIssueProvider.ts +++ b/packages/core/src/codewhisperer/service/securityIssueProvider.ts @@ -6,6 +6,7 @@ import * as vscode from 'vscode' import { AggregatedCodeScanIssue, CodeScanIssue, SuggestedFix } from '../models/model' import { randomUUID } from '../../shared/crypto' +import { displayFindingsDetectorName } from '../models/constants' export class SecurityIssueProvider { static #instance: SecurityIssueProvider @@ -161,6 +162,30 @@ export class SecurityIssueProvider { ) } + public mergeIssuesDisplayFindings(newIssues: AggregatedCodeScanIssue, fromQCA: boolean) { + const existingGroup = this._issues.find((group) => group.filePath === newIssues.filePath) + if (!existingGroup) { + this._issues.push(newIssues) + return + } + + this._issues = this._issues.map((group) => + group.filePath !== newIssues.filePath + ? group + : { + ...group, + issues: [ + ...group.issues.filter( + // if the incoming findings are from QCA review, then keep only existing findings from displayFindings + // if the incoming findings are not from QCA review, then keep only the existing QCA findings + (issue) => fromQCA === (issue.detectorName === displayFindingsDetectorName) + ), + ...newIssues.issues, + ], + } + ) + } + private isExistingIssue(issue: CodeScanIssue, filePath: string) { return this._issues .find((group) => group.filePath === filePath) From e23ebf3d4b4f70b3b9bcbc1661a5ec8410f7f80a Mon Sep 17 00:00:00 2001 From: Blake Lazarine Date: Fri, 1 Aug 2025 11:52:40 -0700 Subject: [PATCH 2/4] fix(amazonq): handle agentic CodeAnalysisScope --- packages/core/src/codewhisperer/commands/startSecurityScan.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/core/src/codewhisperer/commands/startSecurityScan.ts b/packages/core/src/codewhisperer/commands/startSecurityScan.ts index 5ff6d13bd91..bd081face38 100644 --- a/packages/core/src/codewhisperer/commands/startSecurityScan.ts +++ b/packages/core/src/codewhisperer/commands/startSecurityScan.ts @@ -108,6 +108,9 @@ export async function startSecurityScan( zipUtil: ZipUtil = new ZipUtil(), scanUuid?: string ) { + if (scope === CodeAnalysisScope.AGENTIC) { + throw new CreateCodeScanFailedError('Cannot use Agentic scope') + } const profile = AuthUtil.instance.regionProfileManager.activeRegionProfile const logger = getLoggerForScope(scope) /** From 03518382ac65b0faf860358c0a876b9550929d58 Mon Sep 17 00:00:00 2001 From: Blake Lazarine Date: Fri, 1 Aug 2025 14:11:32 -0700 Subject: [PATCH 3/4] fix(amazonq): Add comment explaining displayFindings feature flag --- packages/amazonq/src/lsp/client.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/amazonq/src/lsp/client.ts b/packages/amazonq/src/lsp/client.ts index 16ed5357075..58b5a6ee7e7 100644 --- a/packages/amazonq/src/lsp/client.ts +++ b/packages/amazonq/src/lsp/client.ts @@ -182,6 +182,7 @@ export async function startLanguageServer( modelSelection: true, workspaceFilePath: vscode.workspace.workspaceFile?.fsPath, codeReviewInChat: codeReviewInChat, + // feature flag for displaying findings found not through CodeReview in the Code Issues Panel displayFindings: true, }, window: { From 78f65725126db1f6bbecb18c81af7abcb3673b0a Mon Sep 17 00:00:00 2001 From: Blake Lazarine Date: Fri, 1 Aug 2025 18:10:23 -0700 Subject: [PATCH 4/4] fix(amazonq): add unit test for new mergeIssuesDisplayFindings method --- .../mergeIssuesDisplayFindings.test.ts | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 packages/core/src/test/codewhisperer/mergeIssuesDisplayFindings.test.ts diff --git a/packages/core/src/test/codewhisperer/mergeIssuesDisplayFindings.test.ts b/packages/core/src/test/codewhisperer/mergeIssuesDisplayFindings.test.ts new file mode 100644 index 00000000000..3a8c06a3c7d --- /dev/null +++ b/packages/core/src/test/codewhisperer/mergeIssuesDisplayFindings.test.ts @@ -0,0 +1,88 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +import assert from 'assert' +import { SecurityIssueProvider } from '../../codewhisperer/service/securityIssueProvider' +import { createCodeScanIssue } from './testUtil' +import { displayFindingsDetectorName } from '../../codewhisperer/models/constants' +import { AggregatedCodeScanIssue } from '../../codewhisperer/models/model' + +describe('mergeIssuesDisplayFindings', () => { + let provider: SecurityIssueProvider + const testFilePath = '/test/file.py' + + beforeEach(() => { + provider = Object.create(SecurityIssueProvider.prototype) + provider.issues = [] + }) + + it('should add new issues when no existing group', () => { + const newIssues: AggregatedCodeScanIssue = { + filePath: testFilePath, + issues: [createCodeScanIssue({ findingId: 'new-1' })], + } + + provider.mergeIssuesDisplayFindings(newIssues, true) + + assert.strictEqual(provider.issues.length, 1) + assert.strictEqual(provider.issues[0].filePath, testFilePath) + assert.strictEqual(provider.issues[0].issues.length, 1) + assert.strictEqual(provider.issues[0].issues[0].findingId, 'new-1') + }) + + it('should keep displayFindings when fromQCA is true', () => { + provider.issues = [ + { + filePath: testFilePath, + issues: [ + createCodeScanIssue({ findingId: 'qca-1', detectorName: 'QCA-detector' }), + createCodeScanIssue({ findingId: 'display-1', detectorName: displayFindingsDetectorName }), + ], + }, + ] + + const newIssues: AggregatedCodeScanIssue = { + filePath: testFilePath, + issues: [createCodeScanIssue({ findingId: 'new-qca-1', detectorName: 'QCA-detector' })], + } + + provider.mergeIssuesDisplayFindings(newIssues, true) + + assert.strictEqual(provider.issues.length, 1) + assert.strictEqual(provider.issues[0].issues.length, 2) + + const findingIds = provider.issues[0].issues.map((issue) => issue.findingId) + assert.ok(findingIds.includes('display-1')) + assert.ok(findingIds.includes('new-qca-1')) + assert.ok(!findingIds.includes('qca-1')) + }) + + it('should keep QCA findings when fromQCA is false', () => { + provider.issues = [ + { + filePath: testFilePath, + issues: [ + createCodeScanIssue({ findingId: 'qca-1', detectorName: 'QCA-detector' }), + createCodeScanIssue({ findingId: 'display-1', detectorName: displayFindingsDetectorName }), + ], + }, + ] + + const newIssues: AggregatedCodeScanIssue = { + filePath: testFilePath, + issues: [createCodeScanIssue({ findingId: 'new-display-1', detectorName: displayFindingsDetectorName })], + } + + provider.mergeIssuesDisplayFindings(newIssues, false) + + assert.strictEqual(provider.issues.length, 1) + assert.strictEqual(provider.issues[0].issues.length, 2) + + const findingIds = provider.issues[0].issues.map((issue) => issue.findingId) + assert.ok(findingIds.includes('qca-1')) + assert.ok(findingIds.includes('new-display-1')) + assert.ok(!findingIds.includes('display-1')) + }) +})