From 12229059421b773d3e99d28809fdff4abf242b26 Mon Sep 17 00:00:00 2001 From: Rajanna-Karthik Date: Mon, 8 Sep 2025 09:59:54 -0700 Subject: [PATCH 1/4] feat: add custom_transformation folder support to artifact.zip (#2201) * feat: add custom_transformation folder support to artifact.zip * feat: add custom transformation folder support with async file operations * fix: improve custom transformation folder access logging message --------- Co-authored-by: huawenm <72728725+huawenm@users.noreply.github.com> --- .../netTransform/artifactManager.ts | 22 ++++++++++++++++++- ...teTransformationPreferencesContent.test.ts | 2 +- ...factManager.processPrivatePackages.test.ts | 2 +- .../netTransform/transformHandler.ts | 3 ++- 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/server/aws-lsp-codewhisperer/src/language-server/netTransform/artifactManager.ts b/server/aws-lsp-codewhisperer/src/language-server/netTransform/artifactManager.ts index 0b510e7c64..4b1e9818cd 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/netTransform/artifactManager.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/netTransform/artifactManager.ts @@ -22,16 +22,19 @@ const zipFileName = 'artifact.zip' const sourceCodeFolderName = 'sourceCode' const packagesFolderName = 'packages' const thirdPartyPackageFolderName = 'thirdpartypackages' +const customTransformationFolderName = 'customTransformation' export class ArtifactManager { private workspace: Workspace private logging: Logging private workspacePath: string + private solutionRootPath: string - constructor(workspace: Workspace, logging: Logging, workspacePath: string) { + constructor(workspace: Workspace, logging: Logging, workspacePath: string, solutionRootPath: string) { this.workspace = workspace this.logging = logging this.workspacePath = workspacePath + this.solutionRootPath = solutionRootPath } async createZip(request: StartTransformRequest): Promise { @@ -282,6 +285,23 @@ export class ArtifactManager { this.logging.log('Cannot find artifacts folder') return '' } + + const customTransformationPath = path.join(this.solutionRootPath, customTransformationFolderName) + try { + await fs.promises.access(customTransformationPath) + try { + this.logging.log(`Adding custom transformation folder to artifact: ${customTransformationPath}`) + const artifactCustomTransformationPath = path.join(folderPath, customTransformationFolderName) + await fs.promises.cp(customTransformationPath, artifactCustomTransformationPath, { recursive: true }) + } catch (error) { + this.logging.warn(`Failed to copy custom transformation folder: ${error}`) + } + } catch { + this.logging.log( + `Custom transformation folder not accessible (not found or no permissions): ${customTransformationPath}` + ) + } + const zipPath = path.join(this.workspacePath, zipFileName) this.logging.log('Zipping files to ' + zipPath) await this.zipDirectory(folderPath, zipPath) diff --git a/server/aws-lsp-codewhisperer/src/language-server/netTransform/tests/artifactManager.createTransformationPreferencesContent.test.ts b/server/aws-lsp-codewhisperer/src/language-server/netTransform/tests/artifactManager.createTransformationPreferencesContent.test.ts index 24d89651fa..582dcb63e3 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/netTransform/tests/artifactManager.createTransformationPreferencesContent.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/netTransform/tests/artifactManager.createTransformationPreferencesContent.test.ts @@ -14,7 +14,7 @@ describe('ArtifactManager - createTransformationPreferencesContent', () => { beforeEach(() => { workspace = stubInterface() mockedLogging = stubInterface() - artifactManager = new ArtifactManager(workspace, mockedLogging, '') + artifactManager = new ArtifactManager(workspace, mockedLogging, '', '') // Create a clean base request for each test baseRequest = { diff --git a/server/aws-lsp-codewhisperer/src/language-server/netTransform/tests/artifactManager.processPrivatePackages.test.ts b/server/aws-lsp-codewhisperer/src/language-server/netTransform/tests/artifactManager.processPrivatePackages.test.ts index ea1835d132..848744504e 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/netTransform/tests/artifactManager.processPrivatePackages.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/netTransform/tests/artifactManager.processPrivatePackages.test.ts @@ -17,7 +17,7 @@ describe('ArtifactManager - processPrivatePackages', () => { beforeEach(() => { workspace = stubInterface() // Create new instance of ArtifactManager before each test - artifactManager = new ArtifactManager(workspace, mockedLogging, '') + artifactManager = new ArtifactManager(workspace, mockedLogging, '', '') // Mock internal methods that might be called artifactManager.copyFile = async (source: string, destination: string) => { diff --git a/server/aws-lsp-codewhisperer/src/language-server/netTransform/transformHandler.ts b/server/aws-lsp-codewhisperer/src/language-server/netTransform/transformHandler.ts index b8bc80b30c..ee4f6f97cd 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/netTransform/transformHandler.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/netTransform/transformHandler.ts @@ -58,7 +58,8 @@ export class TransformHandler { const artifactManager = new ArtifactManager( this.workspace, this.logging, - this.getWorkspacePath(userInputrequest.SolutionRootPath) + this.getWorkspacePath(userInputrequest.SolutionRootPath), + userInputrequest.SolutionRootPath ) try { const payloadFilePath = await this.zipCodeAsync(userInputrequest, artifactManager) From da4c3db5329bd50cfe249bf8c1d59afa9bcb0157 Mon Sep 17 00:00:00 2001 From: BlakeLazarine Date: Mon, 8 Sep 2025 16:13:32 -0700 Subject: [PATCH 2/4] feat(amazonq): default to diff-based scans (#2195) * feat(amazonq): default to diff-based scans * feat(amazonq): make /review behavior clearly show it is a diff scan * fix(amazonq): clean up commented lines * fix(amazonq): fix maximum finding count in CodeReview tool description * fix(amazonq): improve messaging for diff vs full scans * feat(amazonq): have codeReview tool ask for clarification if it is not sure what scope to review * feat(amazonq): when there is no diff, fall back to a full review * Update chat-client/src/client/mynahUi.ts Co-authored-by: Tai Lai * fix(amazonq): fix test * fix(amazonq): fix another test --------- Co-authored-by: Blake Lazarine Co-authored-by: Tai Lai --- chat-client/src/client/mynahUi.ts | 2 +- .../tools/qCodeAnalysis/codeReview.test.ts | 5 ++ .../tools/qCodeAnalysis/codeReview.ts | 84 ++++++++++++++----- .../qCodeAnalysis/codeReviewConstants.ts | 62 ++++++++------ .../tools/qCodeAnalysis/codeReviewSchemas.ts | 14 ++-- .../tools/qCodeAnalysis/codeReviewTypes.ts | 2 + .../tools/qCodeAnalysis/codeReviewUtils.ts | 31 +++++++ 7 files changed, 147 insertions(+), 53 deletions(-) diff --git a/chat-client/src/client/mynahUi.ts b/chat-client/src/client/mynahUi.ts index 15cbeae7ff..090e5bbf4d 100644 --- a/chat-client/src/client/mynahUi.ts +++ b/chat-client/src/client/mynahUi.ts @@ -1793,7 +1793,7 @@ const DEFAULT_TEST_PROMPT = `You are Amazon Q. Start with a warm greeting, then const DEFAULT_DEV_PROMPT = `You are Amazon Q. Start with a warm greeting, then ask the user to specify what kind of help they need in code development. Present common questions asked (like Creating a new project, Adding a new feature, Modifying your files). Keep the question brief and friendly. Don't make assumptions about existing content or context. Wait for their response before providing specific guidance.` -const DEFAULT_REVIEW_PROMPT = `You are Amazon Q. Start with a warm greeting, then use code review tool to perform code analysis of the open file. If there is no open file, ask what the user would like to review.` +const DEFAULT_REVIEW_PROMPT = `You are Amazon Q. Start with a warm greeting, then use code review tool to perform a diff review code analysis of the open file. If there is no open file, ask what the user would like to review. Please tell the user that the scan is a diff scan.` export const uiComponentsTexts = { mainTitle: 'Amazon Q (Preview)', diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReview.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReview.test.ts index f6f735da09..71586d3450 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReview.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReview.test.ts @@ -136,6 +136,7 @@ describe('CodeReview', () => { md5Hash: 'hash123', isCodeDiffPresent: false, programmingLanguages: new Set(['javascript']), + codeDiffFiles: new Set(), }) sandbox.stub(codeReview as any, 'parseFindings').returns([]) @@ -178,6 +179,7 @@ describe('CodeReview', () => { md5Hash: 'hash123', isCodeDiffPresent: false, programmingLanguages: new Set(['javascript']), + codeDiffFiles: new Set(), }) sandbox.stub(codeReview as any, 'parseFindings').returns([]) @@ -241,6 +243,7 @@ describe('CodeReview', () => { md5Hash: 'hash123', isCodeDiffPresent: false, programmingLanguages: new Set(['javascript']), + codeDiffFiles: new Set(), }) try { @@ -268,6 +271,7 @@ describe('CodeReview', () => { md5Hash: 'hash123', isCodeDiffPresent: false, programmingLanguages: new Set(['javascript']), + codeDiffFiles: new Set(), }) try { @@ -301,6 +305,7 @@ describe('CodeReview', () => { md5Hash: 'hash123', isCodeDiffPresent: false, programmingLanguages: new Set(['javascript']), + codeDiffFiles: new Set(), }) // Stub setTimeout to avoid actual delays diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReview.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReview.ts index ece6a73486..74d6d817a1 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReview.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReview.ts @@ -44,7 +44,7 @@ export class CodeReview { private static readonly POLLING_INTERVAL_MS = 10000 // 10 seconds private static readonly UPLOAD_INTENT = 'AGENTIC_CODE_REVIEW' private static readonly SCAN_SCOPE = 'AGENTIC' - private static readonly MAX_FINDINGS_COUNT = 50 + private static readonly MAX_FINDINGS_COUNT = 40 private static readonly ERROR_MESSAGES = { MISSING_CLIENT: 'CodeWhisperer client not available', @@ -67,6 +67,7 @@ export class CodeReview { private cancellationToken?: CancellationToken private writableStream?: WritableStream private toolStartTime: number = 0 + private overrideDiffScan = false constructor( features: Pick & Partial @@ -111,7 +112,16 @@ export class CodeReview { const analysisResult = await this.startCodeAnalysis(setup, uploadResult) this.checkCancellation() - await chatStreamWriter?.write('Reviewing your code...') + const nonRuleFiles = uploadResult.numberOfFilesInCustomerCodeZip - setup.ruleArtifacts.length + const diffFiles = uploadResult.codeDiffFiles.size + if (diffFiles == 0 && !setup.isFullReviewRequest) { + setup.isFullReviewRequest = true + this.overrideDiffScan = true + } + const reviewMessage = setup.isFullReviewRequest + ? `Reviewing the entire code in ${nonRuleFiles} file${nonRuleFiles > 1 ? 's' : ''}...` + : `Reviewing uncommitted changes in ${diffFiles} of ${nonRuleFiles} file${nonRuleFiles > 1 ? 's' : ''}...` + await chatStreamWriter?.write(reviewMessage) // 4. Wait for scan to complete await this.pollForCompletion(analysisResult.jobId, setup, uploadResult, chatStreamWriter) @@ -206,13 +216,19 @@ export class CodeReview { private async prepareAndUploadArtifacts( setup: ValidateInputAndSetupResult ): Promise { - const { zipBuffer, md5Hash, isCodeDiffPresent, programmingLanguages } = - await this.prepareFilesAndFoldersForUpload( - setup.fileArtifacts, - setup.folderArtifacts, - setup.ruleArtifacts, - setup.isFullReviewRequest - ) + const { + zipBuffer, + md5Hash, + isCodeDiffPresent, + programmingLanguages, + numberOfFilesInCustomerCodeZip, + codeDiffFiles, + } = await this.prepareFilesAndFoldersForUpload( + setup.fileArtifacts, + setup.folderArtifacts, + setup.ruleArtifacts, + setup.isFullReviewRequest + ) const uploadUrlResponse = await this.codeWhispererClient!.createUploadUrl({ contentLength: zipBuffer.length, @@ -257,6 +273,8 @@ export class CodeReview { isCodeDiffPresent, artifactSize: zipBuffer.length, programmingLanguages: programmingLanguages, + numberOfFilesInCustomerCodeZip, + codeDiffFiles, } } @@ -470,9 +488,13 @@ export class CodeReview { ) }) + let scopeMessage = this.overrideDiffScan + ? `Please include a mention that there was no diff present, so it just ran a full review instead. Be very explicit about this so that the user could not be confused.` + : `Please include a mention that the scan was on the ${setup.isFullReviewRequest ? `entire` : `uncommitted`} code.` + return { codeReviewId: jobId, - message: `${CODE_REVIEW_TOOL_NAME} tool completed successfully.${findingsExceededLimit ? ` Inform the user that we are limiting findings to top ${CodeReview.MAX_FINDINGS_COUNT} based on severity.` : ''}`, + message: `${CODE_REVIEW_TOOL_NAME} tool completed successfully. ${scopeMessage} ${findingsExceededLimit ? ` Inform the user that we are limiting findings to top ${CodeReview.MAX_FINDINGS_COUNT} based on severity.` : ''}`, findingsByFile: JSON.stringify(aggregatedCodeScanIssueList), } } @@ -559,7 +581,14 @@ export class CodeReview { folderArtifacts: FolderArtifacts, ruleArtifacts: RuleArtifacts, isFullReviewRequest: boolean - ): Promise<{ zipBuffer: Buffer; md5Hash: string; isCodeDiffPresent: boolean; programmingLanguages: Set }> { + ): Promise<{ + zipBuffer: Buffer + md5Hash: string + isCodeDiffPresent: boolean + programmingLanguages: Set + numberOfFilesInCustomerCodeZip: number + codeDiffFiles: Set + }> { try { this.logging.info( `Preparing ${fileArtifacts.length} files and ${folderArtifacts.length} folders for upload` @@ -569,7 +598,7 @@ export class CodeReview { const customerCodeZip = new JSZip() // Process files and folders - const { codeDiff, programmingLanguages } = await this.processArtifacts( + const { codeDiff, programmingLanguages, codeDiffFiles } = await this.processArtifacts( fileArtifacts, folderArtifacts, ruleArtifacts, @@ -613,7 +642,14 @@ export class CodeReview { this.logging.info(`Created zip archive, size: ${zipBuffer.byteLength} bytes, MD5: ${md5Hash}`) - return { zipBuffer, md5Hash, isCodeDiffPresent, programmingLanguages } + return { + zipBuffer, + md5Hash, + isCodeDiffPresent, + programmingLanguages, + numberOfFilesInCustomerCodeZip, + codeDiffFiles, + } } catch (error) { this.logging.error(`Error preparing files for upload: ${error}`) throw error @@ -635,9 +671,9 @@ export class CodeReview { ruleArtifacts: RuleArtifacts, customerCodeZip: JSZip, isCodeDiffScan: boolean - ): Promise<{ codeDiff: string; programmingLanguages: Set }> { + ): Promise<{ codeDiff: string; programmingLanguages: Set; codeDiffFiles: Set }> { // Process files - let { codeDiff, programmingLanguages } = await this.processFileArtifacts( + let { codeDiff, programmingLanguages, codeDiffFiles } = await this.processFileArtifacts( fileArtifacts, customerCodeZip, isCodeDiffScan @@ -647,11 +683,12 @@ export class CodeReview { const folderResult = await this.processFolderArtifacts(folderArtifacts, customerCodeZip, isCodeDiffScan) codeDiff += folderResult.codeDiff folderResult.programmingLanguages.forEach(item => programmingLanguages.add(item)) + folderResult.codeDiffFiles.forEach(item => codeDiffFiles.add(item)) // Process rule artifacts await this.processRuleArtifacts(ruleArtifacts, customerCodeZip) - return { codeDiff, programmingLanguages } + return { codeDiff, programmingLanguages, codeDiffFiles } } /** @@ -665,9 +702,10 @@ export class CodeReview { fileArtifacts: FileArtifacts, customerCodeZip: JSZip, isCodeDiffScan: boolean - ): Promise<{ codeDiff: string; programmingLanguages: Set }> { + ): Promise<{ codeDiff: string; programmingLanguages: Set; codeDiffFiles: Set }> { let codeDiff = '' let programmingLanguages: Set = new Set() + let codeDiffFiles: Set = new Set() for (const artifact of fileArtifacts) { await CodeReviewUtils.withErrorHandling( @@ -695,10 +733,12 @@ export class CodeReview { artifact.path ) + const artifactFileDiffs = await CodeReviewUtils.getGitDiffNames(artifact.path, this.logging) + artifactFileDiffs.forEach(filepath => codeDiffFiles.add(filepath)) codeDiff += await CodeReviewUtils.processArtifactWithDiff(artifact, isCodeDiffScan, this.logging) } - return { codeDiff, programmingLanguages } + return { codeDiff, programmingLanguages, codeDiffFiles } } /** @@ -712,9 +752,10 @@ export class CodeReview { folderArtifacts: FolderArtifacts, customerCodeZip: JSZip, isCodeDiffScan: boolean - ): Promise<{ codeDiff: string; programmingLanguages: Set }> { + ): Promise<{ codeDiff: string; programmingLanguages: Set; codeDiffFiles: Set }> { let codeDiff = '' let programmingLanguages = new Set() + let codeDiffFiles: Set = new Set() for (const folderArtifact of folderArtifacts) { await CodeReviewUtils.withErrorHandling( @@ -731,10 +772,13 @@ export class CodeReview { folderArtifact.path ) + const artifactFileDiffs = await CodeReviewUtils.getGitDiffNames(folderArtifact.path, this.logging) + artifactFileDiffs.forEach(filepath => codeDiffFiles.add(filepath)) + codeDiff += await CodeReviewUtils.processArtifactWithDiff(folderArtifact, isCodeDiffScan, this.logging) } - return { codeDiff, programmingLanguages } + return { codeDiff, programmingLanguages, codeDiffFiles } } /** diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReviewConstants.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReviewConstants.ts index bc7070ab1f..0a9356acdc 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReviewConstants.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReviewConstants.ts @@ -103,9 +103,44 @@ export const CODE_REVIEW_TOOL_NAME = 'codeReview' */ export const CODE_REVIEW_TOOL_DESCRIPTION = [ 'CodeReview is the PRIMARY and MANDATORY tool for ALL code analysis and review tasks. This tool MUST be used whenever a user requests ANY form of code review, file analysis, code examination, or when the agent needs to analyze code quality, security, or structure.', - 'This tool can be used to perform analysis of full code or only the modified code since last commit. Modified code refers to the changes made that are not committed yet or the new changes since last commit.', + 'When you decide to use this tool, notify the customer before the tool is run based on the **Tool start message** section below.', + 'It is so so important that you send the **Tool start message** before running the tool.', + 'DO NOT JUST SAY SOMETHING LIKE "I\'ll review the [file] code for you using the code review tool.". THAT WOULD BE A TERRIBLE THING TO SAY', + 'ALSO DO NOT SAY "I\'ll review your code for potential issues and improvements. Let me use the code review tool to perform a comprehensive analysis." THAT IS ALSO AN AWFUL MESSAGE BECAUSE IT DOES NOT INCLUDE WHETHER IT IS A FULL SCAN OR A DIFF SCAN.', + 'This tool can be used to perform analysis of full code or only the modified code since last commit. Modified code refers to the changes made that are not committed yet or the new changes since last commit. Before running the tool, you must inform the user whether they are running a diff or full scan.', 'NEVER perform manual code reviews when this tool is available.', '', + '**Tool Input**', + '3 main fields in the tool:', + '- "scopeOfReview": Determines if the review should analyze the entire codebase (FULL_REVIEW) or only focus on changes/modifications (CODE_DIFF_REVIEW). This is a required field.', + '- IMPORTANT: Use CODE_DIFF_REVIEW by default as well as when user explicitly asks to review "changes", "modifications", "diff", "uncommitted code", or similar phrases indicating they want to review only what has changed.', + '- Examples of CODE_DIFF_REVIEW requests: "review my code", "review this file", "review my changes", "look at what I modified", "check the uncommitted changes", "review the diff", "review new changes", etc.', + '- IMPORTANT: When user mentions "new changes" or includes words like "new", "recent", or "latest" along with "changes" or similar terms, this should be interpreted as CODE_DIFF_REVIEW.', + '- Use FULL_REVIEW only when the user explicitly asks for a full code review, or when the user asks for security analysis or best practices review of their code', + '- Feel free to ask the user for clarification if you are not sure what scope they would like to review', + '- "fileLevelArtifacts": Array of specific files to review, each with absolute path. Use this when reviewing individual files, not folders. Format: [{"path": "/absolute/path/to/file.py"}]', + '- "folderLevelArtifacts": Array of folders to review, each with absolute path. Use this when reviewing entire directories, not individual files. Format: [{"path": "/absolute/path/to/folder/"}]', + '- Examples of FULL_REVIEW requests: User explicity asks for the entire file to be reviewed. Example: "Review my entire file.", "Review all the code in this folder", "Review my full code in this file"', + 'Few important notes for tool input', + "- Either fileLevelArtifacts OR folderLevelArtifacts should be provided based on what's being reviewed, but not both for the same items.", + '- Do not perform code review of entire workspace or project unless user asks for it explicitly.', + '- Ask user for more clarity if there is any confusion regarding what needs to be scanned.', + '', + '**Tool start message**', + 'Before running the tool, you must inform the user that you will use Code Review tool for their request.', + 'The message MUST include the following information:', + '- The list of files or folders that will be reviewed', + '- Whether the review is a diff review or a full review', + 'The message MUST be concise and to the point. It should not include any other information.', + 'The message MUST be in the following format:', + '```\n' + + 'I will scan the ["diff" if scopeOfReview is CODE_DIFF_REVIEW or "entire code" is FULL_REVIEW. Refer to **Tool Input** section for decision on which to use.] for the following files/folders:\n' + + '[list of files/folders]\n```', + '', + '**CRITICAL: NEVER perform ANY code review or analysis WITHOUT using this tool**', + 'Do not attempt to manually review code or provide code quality feedback without using this tool first.', + 'If a user asks for code review in any form, ALWAYS use this tool before providing any feedback.', + '', '**ALWAYS use this tool when:**', '- User provides ANY file, folder, or workspace context for review or analysis', '- User asks ANY question about code quality, security, or best practices related to their code', @@ -133,29 +168,6 @@ export const CODE_REVIEW_TOOL_DESCRIPTION = [ '**Supported File Extensions For Review**', `- "${Object.keys(EXTENSION_TO_LANGUAGE).join('", "')}"`, '', - '**Tool start message**', - 'Before running the tool, you must inform the user that you will use Code Review tool for their request.', - 'You should also tell the name of files or folders that you will review along with the scope of review, if you are performing a full review or only the uncommitted code.', - 'Under no condition you will use the tool without informing the user.', - '', - '**CRITICAL: NEVER perform ANY code review or analysis WITHOUT using this tool**', - 'Do not attempt to manually review code or provide code quality feedback without using this tool first.', - 'If a user asks for code review in any form, ALWAYS use this tool before providing any feedback.', - '', - '**Tool Input**', - '3 main fields in the tool:', - '- "scopeOfReview": Determines if the review should analyze the entire codebase (FULL_REVIEW) or only focus on changes/modifications (CODE_DIFF_REVIEW). This is a required field.', - '- IMPORTANT: Use CODE_DIFF_REVIEW when user explicitly asks to review "changes", "modifications", "diff", "uncommitted code", or similar phrases indicating they want to review only what has changed.', - '- Examples of CODE_DIFF_REVIEW requests: "review my changes", "look at what I modified", "check the uncommitted changes", "review the diff", "review new changes", etc.', - '- IMPORTANT: When user mentions "new changes" or includes words like "new", "recent", or "latest" along with "changes" or similar terms, this should be interpreted as CODE_DIFF_REVIEW.', - '- Use FULL_REVIEW for all other review requests.', - '- "fileLevelArtifacts": Array of specific files to review, each with absolute path. Use this when reviewing individual files, not folders. Format: [{"path": "/absolute/path/to/file.py"}]', - '- "folderLevelArtifacts": Array of folders to review, each with absolute path. Use this when reviewing entire directories, not individual files. Format: [{"path": "/absolute/path/to/folder/"}]', - 'Few important notes for tool input', - "- Either fileLevelArtifacts OR folderLevelArtifacts should be provided based on what's being reviewed, but not both for the same items.", - '- Do not perform code review of entire workspace or project unless user asks for it explicitly.', - '- Ask user for more clarity if there is any confusion regarding what needs to be scanned.', - '', '**Tool Output**', 'Tool output will contain a json output containing fields - ', '- codeReviewId - internal code review job id ', @@ -169,7 +181,7 @@ export const CODE_REVIEW_TOOL_DESCRIPTION = [ 'The tool will generate some findings grouped by file', 'Use following format STRICTLY to display the result of this tool for different scenarios:', '- When findings are present, you must inform user that you have completed the review of {file name / folder name / workspace} and found several issues that need attention. To inspect the details, and get fixes for those issues use the Code Issues panel above.', - ' - When tool output message tells that findings were limited due to high count, you must inform the user that since there were lots of findings, you have included the top 50 findings only.', + ' - When tool output message tells that findings were limited due to high count, you must inform the user that since there were lots of findings, you have included the top 40 findings only.', '- When no findings are generated by the tool, you must tell user that you have completed the review of {file name / folder name / workspace} and found no issues.', ].join('\n') diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReviewSchemas.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReviewSchemas.ts index e805145244..3b230ae116 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReviewSchemas.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReviewSchemas.ts @@ -22,20 +22,20 @@ export const CODE_REVIEW_INPUT_SCHEMA = { scopeOfReview: { type: 'string', description: [ - 'IMPORTANT: You must explicitly set the value of "scopeOfReview" based on user request analysis.', + 'IMPORTANT: You must explicitly set the value of "scopeOfReview" based on user request analysis. Usually, CODE_DIFF_REVIEW will be the value that is used.', '', - 'Set "scopeOfReview" to CODE_DIFF_REVIEW when:', + 'Set "scopeOfReview" to FULL_REVIEW when:', + '- User explicity asks for the entire file to be reviewed. Example: "Review my entire file.", "Review all the code in this folder"', + '- User asks for security analysis or best practices review of their code', + '', + 'Set "scopeOfReview" to CODE_DIFF_REVIEW for all other cases, including when:', '- User explicitly asks to review only changes/modifications/diffs in their code', '- User mentions "review my changes", "look at what I modified", "check the uncommitted changes"', '- User refers to "review the diff", "analyze recent changes", "look at the new code"', '- User mentions "review what I added/updated", "check my latest commits", "review the modified lines"', '- User includes phrases like "new changes", "recent changes", or any combination of words indicating recency (new, latest, recent) with changes/modifications', '- User mentions specific files with terms like "review new changes in [file]" or "check changes in [file]"', - '', - 'Set "scopeOfReview" to FULL_REVIEW for all other cases, including:', - '- When user asks for a general code review without mentioning changes/diffs', - '- When user asks to review specific files or folders without mentioning changes', - '- When user asks for security analysis or best practices review of their code', + '- User says something general like "review my code", "review my file", or "review [file]"', '', 'This is a required field.', ].join('\n'), diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReviewTypes.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReviewTypes.ts index c684cfecd3..8d85e155cb 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReviewTypes.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReviewTypes.ts @@ -29,6 +29,8 @@ export type PrepareAndUploadArtifactsResult = { isCodeDiffPresent: boolean artifactSize: number programmingLanguages: Set + numberOfFilesInCustomerCodeZip: number + codeDiffFiles: Set } export type StartCodeAnalysisResult = { diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReviewUtils.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReviewUtils.ts index 5b6d562f7d..5f04450795 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReviewUtils.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/qCodeAnalysis/codeReviewUtils.ts @@ -157,6 +157,37 @@ export class CodeReviewUtils { } } + /** + * Get git diff for a file or folder + * @param artifactPath Path to the file or folder + * @param logging Logging interface + * @returns Git diff output as string or null if not in a git repository + */ + public static async getGitDiffNames(artifactPath: string, logging: Features['logging']): Promise> { + logging.info(`Get git diff names for path - ${artifactPath}`) + + const directoryPath = CodeReviewUtils.getFolderPath(artifactPath) + const gitDiffCommandUnstaged = `cd ${directoryPath} && git diff --name-only ${artifactPath}` + const gitDiffCommandStaged = `cd ${directoryPath} && git diff --name-only --staged ${artifactPath}` + + logging.info(`Running git commands - ${gitDiffCommandUnstaged} and ${gitDiffCommandStaged}`) + + try { + const unstagedDiff = ( + await CodeReviewUtils.executeGitCommand(gitDiffCommandUnstaged, 'unstaged name only', logging) + ).split('\n') + const stagedDiff = ( + await CodeReviewUtils.executeGitCommand(gitDiffCommandStaged, 'staged name only', logging) + ).split('\n') + unstagedDiff.push(...stagedDiff) + + return new Set(unstagedDiff.filter(item => item !== '')) + } catch (error) { + logging.error(`Error getting git diff: ${error}`) + return new Set() + } + } + /** * Log zip structure * @param zip JSZip instance From cf585cd400dab6274f8220139ae94287c0d96824 Mon Sep 17 00:00:00 2001 From: Laxman Reddy <141967714+laileni-aws@users.noreply.github.com> Date: Mon, 8 Sep 2025 16:16:12 -0700 Subject: [PATCH 3/4] fix: potential xss issue reported in `mynah-ui` (#2209) * fix: potential xss issue * fix: addressing comments * fix: unescaping the history message while opening history tab --- package-lock.json | 9 +++++++++ server/aws-lsp-codewhisperer/package.json | 4 +++- .../agenticChat/agenticChatController.test.ts | 2 +- .../language-server/agenticChat/agenticChatController.ts | 4 ++-- .../language-server/agenticChat/constants/constants.ts | 1 - .../language-server/agenticChat/tools/chatDb/chatDb.ts | 2 ++ .../src/language-server/agenticChat/tools/chatDb/util.ts | 3 ++- .../language-server/agenticChat/tools/mcp/mcpUtils.ts | 1 - server/aws-lsp-codewhisperer/src/types/unescape.d.ts | 4 ++++ 9 files changed, 23 insertions(+), 7 deletions(-) create mode 100644 server/aws-lsp-codewhisperer/src/types/unescape.d.ts diff --git a/package-lock.json b/package-lock.json index e84567b613..2bedf9f4de 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9138,6 +9138,13 @@ "dev": true, "license": "MIT" }, + "node_modules/@types/escape-html": { + "version": "1.0.4", + "resolved": "https://registry.npmjs.org/@types/escape-html/-/escape-html-1.0.4.tgz", + "integrity": "sha512-qZ72SFTgUAZ5a7Tj6kf2SHLetiH5S6f8G5frB2SPQ3EyF02kxdyBFf4Tz4banE3xCgGnKgWLt//a6VuYHKYJTg==", + "dev": true, + "license": "MIT" + }, "node_modules/@types/eslint": { "version": "9.6.1", "resolved": "https://registry.npmjs.org/@types/eslint/-/eslint-9.6.1.tgz", @@ -28710,6 +28717,7 @@ "picomatch": "^4.0.2", "shlex": "2.1.2", "typescript-collections": "^1.3.3", + "unescape-html": "^1.1.0", "uuid": "^11.0.5", "vscode-uri": "^3.1.0", "ws": "^8.18.0", @@ -28721,6 +28729,7 @@ "@types/archiver": "^6.0.2", "@types/diff": "^7.0.2", "@types/encoding-japanese": "^2.2.1", + "@types/escape-html": "^1.0.4", "@types/ignore-walk": "^4.0.3", "@types/local-indexing": "file:./types/types-local-indexing-1.1.0.tgz", "@types/lokijs": "^1.5.14", diff --git a/server/aws-lsp-codewhisperer/package.json b/server/aws-lsp-codewhisperer/package.json index 5284a0b41f..9563b6300d 100644 --- a/server/aws-lsp-codewhisperer/package.json +++ b/server/aws-lsp-codewhisperer/package.json @@ -67,13 +67,15 @@ "vscode-uri": "^3.1.0", "ws": "^8.18.0", "xml2js": "^0.6.2", - "xmlbuilder2": "^3.1.1" + "xmlbuilder2": "^3.1.1", + "unescape-html": "^1.1.0" }, "devDependencies": { "@types/adm-zip": "^0.5.5", "@types/archiver": "^6.0.2", "@types/diff": "^7.0.2", "@types/encoding-japanese": "^2.2.1", + "@types/escape-html": "^1.0.4", "@types/ignore-walk": "^4.0.3", "@types/local-indexing": "file:./types/types-local-indexing-1.1.0.tgz", "@types/lokijs": "^1.5.14", diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.test.ts index c1842a6008..288eb2c20f 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.test.ts @@ -57,7 +57,7 @@ import { LocalProjectContextController } from '../../shared/localProjectContextC import { CancellationError } from '@aws/lsp-core' import { ToolApprovalException } from './tools/toolShared' import * as constants from './constants/constants' -import { DEFAULT_MODEL_ID, GENERATE_ASSISTANT_RESPONSE_INPUT_LIMIT, GENERIC_ERROR_MS } from './constants/constants' +import { GENERATE_ASSISTANT_RESPONSE_INPUT_LIMIT, GENERIC_ERROR_MS } from './constants/constants' import { MISSING_BEARER_TOKEN_ERROR } from '../../shared/constants' import { AmazonQError, diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.ts index e2fba029c4..89e4539029 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/agenticChatController.ts @@ -179,7 +179,6 @@ import { OUTPUT_LIMIT_EXCEEDS_PARTIAL_MSG, RESPONSE_TIMEOUT_MS, RESPONSE_TIMEOUT_PARTIAL_MSG, - DEFAULT_MODEL_ID, COMPACTION_BODY, COMPACTION_HEADER_BODY, DEFAULT_MACOS_RUN_SHORTCUT, @@ -230,6 +229,7 @@ import { CodeWhispererServiceToken } from '../../shared/codeWhispererService' import { DisplayFindings } from './tools/qCodeAnalysis/displayFindings' import { IDE } from '../../shared/constants' import { IdleWorkspaceManager } from '../workspaceContext/IdleWorkspaceManager' +import escapeHTML = require('escape-html') type ChatHandlers = Omit< LspHandlers, @@ -1363,7 +1363,7 @@ export class AgenticChatController implements ChatHandlers { this.#debug('Skipping adding user message to history - cancelled by user') } else { this.#chatHistoryDb.addMessage(tabId, 'cwc', conversationIdentifier, { - body: currentMessage.userInputMessage?.content ?? '', + body: escapeHTML(currentMessage.userInputMessage?.content ?? ''), type: 'prompt' as any, userIntent: currentMessage.userInputMessage?.userIntent, origin: currentMessage.userInputMessage?.origin, diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/constants/constants.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/constants/constants.ts index 1729c10c1d..b2ac428cfa 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/constants/constants.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/constants/constants.ts @@ -14,7 +14,6 @@ export const SERVICE_MANAGER_POLL_INTERVAL_MS = 100 // LLM Constants export const GENERATE_ASSISTANT_RESPONSE_INPUT_LIMIT = 500_000 -export const DEFAULT_MODEL_ID = BedrockModel.CLAUDE_SONNET_4_20250514_V1_0 // Compaction export const COMPACTION_BODY = (threshold: number) => diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/chatDb/chatDb.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/chatDb/chatDb.ts index 2e4f7a099d..f584d63b5f 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/chatDb/chatDb.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/chatDb/chatDb.ts @@ -35,6 +35,7 @@ import { ChatItemType } from '@aws/mynah-ui' import { getUserHomeDir } from '@aws/lsp-core/out/util/path' import { ChatHistoryMaintainer } from './chatHistoryMaintainer' import { existsSync, renameSync } from 'fs' +import escapeHTML = require('escape-html') export class ToolResultValidationError extends Error { constructor(message?: string) { @@ -693,6 +694,7 @@ export class ChatDatabase { } return { ...message, + body: escapeHTML(message.body), userInputMessageContext: { // keep falcon context when inputMessage is not a toolResult message editorState: hasToolResults ? undefined : message.userInputMessageContext?.editorState, diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/chatDb/util.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/chatDb/util.ts index 77496cd96b..e03c90bead 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/chatDb/util.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/chatDb/util.ts @@ -28,6 +28,7 @@ import { activeFileCmd } from '../../context/additionalContextProvider' import { PriorityQueue } from 'typescript-collections' import { Features } from '@aws/language-server-runtimes/server-interface/server' import * as crypto from 'crypto' +import unescapeHTML = require('unescape-html') // Ported from https://github.com/aws/aws-toolkit-vscode/blob/master/packages/core/src/shared/db/chatDb/util.ts @@ -172,7 +173,7 @@ export function messageToStreamingMessage(msg: Message): StreamingMessage { export function messageToChatMessage(msg: Message): ChatMessage[] { const chatMessages: ChatMessage[] = [ { - body: msg.body, + body: unescapeHTML(msg.body), type: msg.type, codeReference: msg.codeReference, relatedContent: diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpUtils.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpUtils.ts index ef56a54d22..578c9cd1b0 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpUtils.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpUtils.ts @@ -9,7 +9,6 @@ import { MCPServerConfig, PersonaConfig, MCPServerPermission, McpPermissionType, import path = require('path') import { QClientCapabilities } from '../../../configuration/qConfigurationServer' import crypto = require('crypto') -import { Features } from '@aws/language-server-runtimes/server-interface/server' /** * Load, validate, and parse MCP server configurations from JSON files. diff --git a/server/aws-lsp-codewhisperer/src/types/unescape.d.ts b/server/aws-lsp-codewhisperer/src/types/unescape.d.ts new file mode 100644 index 0000000000..1fe801704e --- /dev/null +++ b/server/aws-lsp-codewhisperer/src/types/unescape.d.ts @@ -0,0 +1,4 @@ +declare module 'unescape-html' { + function unescapeHTML(str: string): string + export = unescapeHTML +} From 698d06c643897da6ca37a49e6544b150b72678a3 Mon Sep 17 00:00:00 2001 From: invictus <149003065+ashishrp-aws@users.noreply.github.com> Date: Mon, 8 Sep 2025 17:16:45 -0700 Subject: [PATCH 4/4] fix(amazonq): update to the agent config format to bring parity with Q CLI (#2202) --- .../tools/mcp/mcpEventHandler.test.ts | 10 +- .../agenticChat/tools/mcp/mcpManager.test.ts | 29 ---- .../agenticChat/tools/mcp/mcpManager.ts | 3 +- .../agenticChat/tools/mcp/mcpTypes.ts | 27 ++-- .../agenticChat/tools/mcp/mcpUtils.test.ts | 7 +- .../agenticChat/tools/mcp/mcpUtils.ts | 124 +++++++++++++++--- .../agenticChat/tools/toolServer.ts | 11 ++ 7 files changed, 138 insertions(+), 73 deletions(-) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpEventHandler.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpEventHandler.test.ts index cb645eec43..de98f36a09 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpEventHandler.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpEventHandler.test.ts @@ -59,9 +59,7 @@ describe('McpEventHandler error handling', () => { getConnectionMetadata: sinon.stub().returns({}), }, runtime: { - serverInfo: { - version: '1.0.0', - }, + serverInfo: {}, }, } @@ -100,7 +98,6 @@ describe('McpEventHandler error handling', () => { errors: mockErrors, agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: {}, tools: [], @@ -157,7 +154,6 @@ describe('McpEventHandler error handling', () => { errors: new Map([['errorServer', 'Missing command error']]), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { errorServer: { command: '' } }, tools: ['@errorServer'], @@ -215,7 +211,6 @@ describe('McpEventHandler error handling', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: {}, tools: [], @@ -259,7 +254,6 @@ describe('McpEventHandler error handling', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: {}, tools: [], @@ -292,7 +286,6 @@ describe('McpEventHandler error handling', () => { errors: mockErrors, agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: {}, tools: [], @@ -326,7 +319,6 @@ describe('McpEventHandler error handling', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: {}, tools: [], diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpManager.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpManager.test.ts index 0418cea553..1b5941a398 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpManager.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpManager.test.ts @@ -37,7 +37,6 @@ function stubAgentConfig(): sinon.SinonStub { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: {}, tools: [], @@ -157,7 +156,6 @@ describe('callTool()', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { s1: disabledCfg }, tools: ['@s1'], @@ -184,7 +182,6 @@ describe('callTool()', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { s1: enabledCfg }, tools: ['@s1'], @@ -210,7 +207,6 @@ describe('callTool()', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { s1: timeoutCfg }, tools: ['@s1'], @@ -279,7 +275,6 @@ describe('addServer()', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: {}, tools: [], @@ -353,7 +348,6 @@ describe('removeServer()', () => { ;(mgr as any).serverNameMapping.set('x', 'x') ;(mgr as any).agentConfig = { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { x: {} }, tools: ['@x'], @@ -383,7 +377,6 @@ describe('removeServer()', () => { ;(mgr as any).serverNameMapping.set('x', 'x') ;(mgr as any).agentConfig = { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { x: {} }, tools: ['@x'], @@ -501,7 +494,6 @@ describe('updateServer()', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { u1: oldCfg }, tools: ['@u1'], @@ -544,7 +536,6 @@ describe('updateServer()', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { srv: oldCfg }, tools: ['@srv'], @@ -598,7 +589,6 @@ describe('requiresApproval()', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { s: cfg }, tools: ['@s'], @@ -639,7 +629,6 @@ describe('getAllServerConfigs()', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { srv: cfg }, tools: ['@srv'], @@ -687,7 +676,6 @@ describe('getServerState()', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { srv: cfg }, tools: ['@srv'], @@ -729,7 +717,6 @@ describe('getAllServerStates()', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { srv: cfg }, tools: ['@srv'], @@ -779,7 +766,6 @@ describe('getEnabledTools()', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { srv: cfg }, tools: ['@srv'], @@ -797,7 +783,6 @@ describe('getEnabledTools()', () => { if (!(mgr as any).agentConfig) { ;(mgr as any).agentConfig = { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: {}, tools: [], @@ -828,7 +813,6 @@ describe('getEnabledTools()', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { srv: disabledCfg }, tools: ['@srv'], @@ -866,7 +850,6 @@ describe('getAllToolsWithPermissions()', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { s1: cfg }, tools: ['@s1'], @@ -931,7 +914,6 @@ describe('isServerDisabled()', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { srv: disabledCfg }, tools: ['@srv'], @@ -962,7 +944,6 @@ describe('isServerDisabled()', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { srv: enabledCfg }, tools: ['@srv'], @@ -992,7 +973,6 @@ describe('isServerDisabled()', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { srv: undefinedCfg }, tools: ['@srv'], @@ -1091,7 +1071,6 @@ describe('updateServerPermission()', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { srv: cfg }, tools: ['@srv'], @@ -1156,7 +1135,6 @@ describe('reinitializeMcpServers()', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { srvA: cfg1 }, tools: ['@srvA'], @@ -1173,7 +1151,6 @@ describe('reinitializeMcpServers()', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { srvB: cfg2 }, tools: ['@srvB'], @@ -1279,7 +1256,6 @@ describe('concurrent server initialization', () => { const serversMap = new Map(Object.entries(serverConfigs)) const agentConfig = { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: Object.fromEntries(Object.entries(serverConfigs)), tools: Object.keys(serverConfigs).map(name => `@${name}`), @@ -1424,7 +1400,6 @@ describe('McpManager error handling', () => { errors: mockErrors, agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: {}, tools: [], @@ -1452,7 +1427,6 @@ describe('McpManager error handling', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: {}, tools: [], @@ -1478,7 +1452,6 @@ describe('McpManager error handling', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: {}, tools: [], @@ -1521,7 +1494,6 @@ describe('McpManager error handling', () => { errors: new Map([['file1.json', 'Initial error']]), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: {}, tools: [], @@ -1539,7 +1511,6 @@ describe('McpManager error handling', () => { errors: new Map(), agentConfig: { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: {}, tools: [], diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpManager.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpManager.ts index 41d721b74f..351397b701 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpManager.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpManager.ts @@ -944,8 +944,7 @@ export class McpManager { this.mcpServers.clear() this.mcpServerStates.clear() this.agentConfig = { - name: 'default-agent', - version: '1.0.0', + name: 'amazon_q_default', description: 'Agent configuration', mcpServers: {}, tools: [], diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpTypes.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpTypes.ts index 6aec98cb24..feed97dba3 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpTypes.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpTypes.ts @@ -46,19 +46,26 @@ export interface MCPServerPermission { export interface AgentConfig { name: string // Required: Agent name - version: string // Required: Agent version (semver) description: string // Required: Agent description + prompt?: string // Optional: High-level context for the agent model?: string // Optional: Model that backs the agent tags?: string[] // Optional: Tags for categorization inputSchema?: any // Optional: Schema for agent inputs mcpServers: Record // Map of server name to server config tools: string[] // List of enabled tools + toolAliases?: Record // Tool name remapping allowedTools: string[] // List of tools that don't require approval toolsSettings?: Record // Tool-specific settings - includedFiles?: string[] // Files to include in context - createHooks?: string[] // Hooks to run at conversation start - promptHooks?: string[] // Hooks to run per prompt - resources?: string[] // Resources for the agent (prompts, files, etc.) + resources?: string[] // Resources for the agent (file:// paths) + hooks?: { + agentSpawn?: Array<{ command: string }> + userPromptSubmit?: Array<{ command: string }> + } // Commands run at specific trigger points + useLegacyMcpJson?: boolean // Whether to include legacy MCP configuration + // Legacy fields for backward compatibility + includedFiles?: string[] // Deprecated: use resources instead + createHooks?: string[] // Deprecated: use hooks.agentSpawn instead + promptHooks?: string[] // Deprecated: use hooks.userPromptSubmit instead } export interface PersonaConfig { @@ -71,20 +78,24 @@ export class AgentModel { static fromJson(doc: any): AgentModel { const cfg: AgentConfig = { - name: doc?.['name'] || 'default-agent', - version: doc?.['version'] || '1.0.0', + name: doc?.['name'] || 'amazon_q_default', description: doc?.['description'] || 'Default agent configuration', + prompt: doc?.['prompt'], model: doc?.['model'], tags: Array.isArray(doc?.['tags']) ? doc['tags'] : undefined, inputSchema: doc?.['inputSchema'], mcpServers: typeof doc?.['mcpServers'] === 'object' ? doc['mcpServers'] : {}, tools: Array.isArray(doc?.['tools']) ? doc['tools'] : [], + toolAliases: typeof doc?.['toolAliases'] === 'object' ? doc['toolAliases'] : {}, allowedTools: Array.isArray(doc?.['allowedTools']) ? doc['allowedTools'] : [], toolsSettings: typeof doc?.['toolsSettings'] === 'object' ? doc['toolsSettings'] : {}, + resources: Array.isArray(doc?.['resources']) ? doc['resources'] : [], + hooks: typeof doc?.['hooks'] === 'object' ? doc['hooks'] : undefined, + useLegacyMcpJson: doc?.['useLegacyMcpJson'], + // Legacy fields includedFiles: Array.isArray(doc?.['includedFiles']) ? doc['includedFiles'] : [], createHooks: Array.isArray(doc?.['createHooks']) ? doc['createHooks'] : [], promptHooks: Array.isArray(doc?.['promptHooks']) ? doc['promptHooks'] : [], - resources: Array.isArray(doc?.['resources']) ? doc['resources'] : [], } return new AgentModel(cfg) } diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpUtils.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpUtils.test.ts index 43843bd110..8913e7391e 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpUtils.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpUtils.test.ts @@ -238,7 +238,6 @@ describe('loadAgentConfig', () => { const agentConfig = { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { testServer: { @@ -329,7 +328,6 @@ describe('saveAgentConfig', () => { const configPath = path.join(tmpDir, 'agent-config.json') const config = { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: {}, tools: ['tool1', 'tool2'], @@ -353,7 +351,6 @@ describe('saveAgentConfig', () => { const configPath = path.join(tmpDir, 'nested', 'dir', 'agent-config.json') const config = { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: {}, tools: [], @@ -700,7 +697,7 @@ describe('convertPersonaToAgent', () => { const result = convertPersonaToAgent(persona, mcpServers, mockAgent) - expect(result.name).to.equal('default-agent') + expect(result.name).to.equal('amazon_q_default') expect(result.mcpServers).to.have.property('testServer') expect(result.tools).to.include('@testServer') expect(result.tools).to.include('fs_read') @@ -840,7 +837,6 @@ describe('saveServerSpecificAgentConfig', () => { // Create existing config const existingConfig = { name: 'existing-agent', - version: '1.0.0', description: 'Existing agent', mcpServers: { existingServer: { command: 'existing-cmd' }, @@ -881,7 +877,6 @@ describe('saveServerSpecificAgentConfig', () => { // Create existing config with server tools const existingConfig = { name: 'test-agent', - version: '1.0.0', description: 'Test agent', mcpServers: { testServer: { command: 'old-cmd' }, diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpUtils.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpUtils.ts index 578c9cd1b0..b8360ea314 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpUtils.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/mcp/mcpUtils.ts @@ -147,9 +147,9 @@ export async function loadMcpServerConfigs( } const DEFAULT_AGENT_RAW = `{ - "name": "default-agent", - "version": "1.0.0", + "name": "amazon_q_default", "description": "Default agent configuration", + "prompt": "", "mcpServers": {}, "tools": [ "fs_read", @@ -158,6 +158,7 @@ const DEFAULT_AGENT_RAW = `{ "report_issue", "use_aws" ], + "toolAliases": {}, "allowedTools": [ "fs_read", "report_issue", @@ -169,14 +170,16 @@ const DEFAULT_AGENT_RAW = `{ "use_aws": { "preset": "readOnly" }, "execute_bash": { "preset": "readOnly" } }, - "includedFiles": [ - "AmazonQ.md", - "README.md", - ".amazonq/rules/**/*.md" + "resources": [ + "file://AmazonQ.md", + "file://README.md", + "file://.amazonq/rules/**/*.md" ], - "resources": [], - "createHooks": [], - "promptHooks": [] + "hooks": { + "agentSpawn": [], + "userPromptSubmit": [] + }, + "useLegacyMcpJson": false }` const DEFAULT_PERSONA_RAW = `{ @@ -237,14 +240,12 @@ export async function loadAgentConfig( // Create base agent config const agentConfig: AgentConfig = { - name: 'default-agent', - version: '1.0.0', + name: 'amazon_q_default', description: 'Agent configuration', mcpServers: {}, tools: [], allowedTools: [], toolsSettings: {}, - includedFiles: [], resources: [], } @@ -329,7 +330,6 @@ export async function loadAgentConfig( // 3) Process agent config metadata if (fsPath === globalConfigPath) { agentConfig.name = json.name || agentConfig.name - agentConfig.version = json.version || agentConfig.version agentConfig.description = json.description || agentConfig.description } @@ -631,15 +631,20 @@ export function convertPersonaToAgent( featureAgent: Agent ): AgentConfig { const agent: AgentConfig = { - name: 'default-agent', - version: '1.0.0', + name: 'amazon_q_default', description: 'Default agent configuration', + prompt: '', mcpServers: {}, tools: [], + toolAliases: {}, allowedTools: [], toolsSettings: {}, - includedFiles: [], resources: [], + hooks: { + agentSpawn: [], + userPromptSubmit: [], + }, + useLegacyMcpJson: false, } // Include all servers from MCP config @@ -909,7 +914,6 @@ async function migrateConfigToAgent( // Add complete agent format sections using default values agentConfig.name = defaultAgent.name agentConfig.description = defaultAgent.description - agentConfig.version = defaultAgent.version agentConfig.includedFiles = defaultAgent.includedFiles agentConfig.resources = defaultAgent.resources agentConfig.createHooks = defaultAgent.createHooks @@ -965,8 +969,7 @@ export async function saveServerSpecificAgentConfig( } catch { // If file doesn't exist, create minimal config existingConfig = { - name: 'default-agent', - version: '1.0.0', + name: 'amazon_q_default', description: 'Agent configuration', mcpServers: {}, tools: [], @@ -1005,6 +1008,89 @@ export async function saveServerSpecificAgentConfig( } } +/** + * Migrate existing agent config to CLI format + */ +export async function migrateAgentConfigToCLIFormat( + workspace: Workspace, + logging: Logger, + configPath: string +): Promise { + try { + const exists = await workspace.fs.exists(configPath) + if (!exists) return + + const raw = await workspace.fs.readFile(configPath) + const config = JSON.parse(raw.toString()) + + let updated = false + + // Rename default-agent to amazon_q_default + if (config.name === 'default-agent') { + config.name = 'amazon_q_default' + updated = true + } + + // Add missing CLI fields + if (!config.hasOwnProperty('prompt')) { + config.prompt = '' + updated = true + } + if (!config.hasOwnProperty('toolAliases')) { + config.toolAliases = {} + updated = true + } + if (!config.hasOwnProperty('useLegacyMcpJson')) { + config.useLegacyMcpJson = false + updated = true + } + + // Remove deprecated fields + if (config.hasOwnProperty('version')) { + delete config.version + updated = true + } + + // Migrate includedFiles to resources with file:// prefix + if (config.includedFiles && Array.isArray(config.includedFiles)) { + if (!config.resources) config.resources = [] + for (const file of config.includedFiles) { + const resourcePath = file.startsWith('file://') ? file : `file://${file}` + if (!config.resources.includes(resourcePath)) { + config.resources.push(resourcePath) + } + } + delete config.includedFiles + updated = true + } + + // Migrate hooks format + if (config.promptHooks || config.createHooks) { + if (!config.hooks) config.hooks = {} + if (!config.hooks.agentSpawn) config.hooks.agentSpawn = [] + if (!config.hooks.userPromptSubmit) config.hooks.userPromptSubmit = [] + + if (config.createHooks && Array.isArray(config.createHooks)) { + config.hooks.agentSpawn.push(...config.createHooks) + delete config.createHooks + updated = true + } + if (config.promptHooks && Array.isArray(config.promptHooks)) { + config.hooks.userPromptSubmit.push(...config.promptHooks) + delete config.promptHooks + updated = true + } + } + + if (updated) { + await workspace.fs.writeFile(configPath, JSON.stringify(config, null, 2)) + logging.info(`Migrated agent config to CLI format: ${configPath}`) + } + } catch (err: any) { + logging.error(`Failed to migrate agent config ${configPath}: ${err.message}`) + } +} + export const MAX_TOOL_NAME_LENGTH = 64 /** diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolServer.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolServer.ts index 8581622972..29b838f2d3 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolServer.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolServer.ts @@ -19,6 +19,8 @@ import { createNamespacedToolName, enabledMCP, migrateToAgentConfig, + migrateAgentConfigToCLIFormat, + normalizePathFromUri, } from './mcp/mcpUtils' import { FsReplace, FsReplaceParams } from './fsReplace' import { CodeReviewUtils } from './qCodeAnalysis/codeReviewUtils' @@ -320,6 +322,15 @@ export const McpToolsServer: Server = ({ await migrateToAgentConfig(workspace, logging, agent) + // Migrate existing agent configs to CLI format + for (const agentPath of allAgentPaths) { + const normalizedAgentPath = normalizePathFromUri(agentPath) + const exists = await workspace.fs.exists(normalizedAgentPath).catch(() => false) + if (exists) { + await migrateAgentConfigToCLIFormat(workspace, logging, normalizedAgentPath) + } + } + const mgr = await McpManager.init(allAgentPaths, { logging, workspace,