From 1a454009b20a4b39db7c399667bd4a88b1b04cfa Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 31 Mar 2025 14:48:15 -0400 Subject: [PATCH 01/20] feat: port fsRead with minimal changes --- .../agenticChat/tools/fsRead.test.ts | 132 ++++++++++++++++ .../agenticChat/tools/fsRead.ts | 143 ++++++++++++++++++ .../agenticChat/tools/toolShared.ts | 28 ++++ 3 files changed, 303 insertions(+) create mode 100644 server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts create mode 100644 server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts create mode 100644 server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.ts diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts new file mode 100644 index 0000000000..1c6c5b4e4e --- /dev/null +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts @@ -0,0 +1,132 @@ +import * as assert from 'assert' +import { FsRead } from './fsRead' +import * as path from 'path' +import * as fs from 'fs/promises' +import * as os from 'os' +import { TestFeatures } from '@aws/language-server-runtimes/testing' +import { Workspace } from '@aws/language-server-runtimes/server-interface' +import { StubbedInstance } from 'ts-sinon' + +describe('FsRead Tool', () => { + let features: TestFeatures + let testFolder: TestFolder + + before(async () => { + features = new TestFeatures() + features.workspace = { + // @ts-ignore reading a file should use readOnly fs operations + fs: { + readFile: (path, options?) => + fs.readFile(path, { encoding: (options?.encoding || 'utf-8') as BufferEncoding }), + readdir: path => fs.readdir(path, { withFileTypes: true }), + exists: path => + fs + .access(path) + .then(() => true) + .catch(() => false), + } as Workspace['fs'], + } as StubbedInstance + testFolder = await TestFolder.create() + }) + + after(async () => { + testFolder.delete() + }) + + afterEach(async () => { + testFolder.clear() + }) + + it('throws if path is empty', async () => { + const fsRead = new FsRead(features, { path: '', homePath: os.homedir() }) + await assert.rejects(fsRead.validate(), /Path cannot be empty/i, 'Expected an error about empty path') + }) + + it('reads entire file', async () => { + const fileContent = 'Line 1\nLine 2\nLine 3' + const filePath = await testFolder.write('fullFile.txt', fileContent) + + const fsRead = new FsRead(features, { path: filePath, homePath: os.homedir() }) + await fsRead.validate() + const result = await fsRead.invoke(process.stdout) + + assert.strictEqual(result.output.kind, 'text', 'Output kind should be "text"') + assert.strictEqual(result.output.content, fileContent, 'File content should match exactly') + }) + + it('reads partial lines of a file', async () => { + const fileContent = 'A\nB\nC\nD\nE\nF' + const filePath = await testFolder.write('partialFile.txt', fileContent) + + const fsRead = new FsRead(features, { path: filePath, readRange: [2, 4], homePath: os.homedir() }) + await fsRead.validate() + const result = await fsRead.invoke(process.stdout) + + assert.strictEqual(result.output.kind, 'text') + assert.strictEqual(result.output.content, 'B\nC\nD') + }) + + it('throws error if path does not exist', async () => { + const filePath = path.join(testFolder.folderPath, 'no_such_file.txt') + const fsRead = new FsRead(features, { path: filePath, homePath: os.homedir() }) + + await assert.rejects( + fsRead.validate(), + /does not exist or cannot be accessed/i, + 'Expected an error indicating the path does not exist' + ) + }) + + it('throws error if content exceeds 30KB', async function () { + const bigContent = 'x'.repeat(35_000) + + const filePath = await testFolder.write('bigFile.txt', bigContent) + + const fsRead = new FsRead(features, { homePath: os.homedir(), path: filePath }) + await fsRead.validate() + + await assert.rejects( + fsRead.invoke(process.stdout), + /This tool only supports reading \d+ bytes at a time/i, + 'Expected a size-limit error' + ) + }) + + it('invalid line range', async () => { + const filePath = await testFolder.write('rangeTest.txt', '1\n2\n3') + const fsRead = new FsRead(features, { path: filePath, readRange: [3, 2], homePath: os.homedir() }) + + await fsRead.validate() + const result = await fsRead.invoke(process.stdout) + assert.strictEqual(result.output.kind, 'text') + assert.strictEqual(result.output.content, '') + }) +}) + +// TODO: move this a test utility file. +class TestFolder { + private constructor(public readonly folderPath: string) {} + + async write(fileName: string, content: string): Promise { + const filePath = path.join(this.folderPath, fileName) + await fs.writeFile(filePath, content) + return filePath + } + + static async create() { + const tempDir = path.join(os.type() === 'Darwin' ? '/tmp' : os.tmpdir(), 'aws-language-servers') + await fs.mkdir(tempDir, { recursive: true }) + return new TestFolder(tempDir) + } + + async delete() { + fs.rm(this.folderPath, { recursive: true, force: true }) + } + + async clear() { + const files = await fs.readdir(this.folderPath) + for (const f of files) { + await fs.rm(path.join(this.folderPath, f), { recursive: true, force: true }) + } + } +} diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts new file mode 100644 index 0000000000..fcfbdf2c9a --- /dev/null +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts @@ -0,0 +1,143 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ +import { InvokeOutput, maxToolResponseSize, OutputKind, sanitizePath } from './toolShared' +import { Writable } from 'stream' +import * as path from 'path' +import { Features } from '@aws/language-server-runtimes/server-interface/server' + +export interface FsReadParams { + path: string + homePath: string + readRange?: number[] +} + +export class FsRead { + private fsPath: string + private readonly homePath: string + private readonly readRange?: number[] + private readonly logging: Features['logging'] + private readonly workspace: Features['workspace'] + + constructor(features: Pick, params: FsReadParams) { + this.fsPath = params.path + this.homePath = params.homePath + this.readRange = params.readRange + this.logging = features.logging + this.workspace = features.workspace + } + + public async validate(): Promise { + this.logging.log(`Validating fsPath: ${this.fsPath}`) + if (!this.fsPath || this.fsPath.trim().length === 0) { + throw new Error('Path cannot be empty.') + } + + const sanitized = sanitizePath(this.fsPath, this.homePath) + this.fsPath = sanitized + + let fileExists: boolean + try { + fileExists = await this.workspace.fs.exists(this.fsPath) + if (!fileExists) { + throw new Error(`Path: "${this.fsPath}" does not exist or cannot be accessed.`) + } + } catch (err) { + throw new Error(`Path: "${this.fsPath}" does not exist or cannot be accessed. (${err})`) + } + + this.logging.log(`Validation succeeded for path: ${this.fsPath}`) + } + + public queueDescription(updates: Writable): void { + const fileName = path.basename(this.fsPath) + updates.write(`Reading file: [${fileName}](${this.fsPath}), `) + + const [start, end] = this.readRange ?? [] + + if (start && end) { + updates.write(`from line ${start} to ${end}`) + } else if (start) { + if (start > 0) { + updates.write(`from line ${start} to end of file`) + } else { + updates.write(`${start} line from the end of file to end of file`) + } + } else { + updates.write('all lines') + } + updates.end() + } + + public async invoke(_updates: Writable): Promise { + try { + const fileContents = await this.readFile(this.fsPath) + this.logging.info(`Read file: ${this.fsPath}, size: ${fileContents.length}`) + return this.handleFileRange(fileContents) + } catch (error: any) { + this.logging.error(`Failed to read "${this.fsPath}": ${error.message || error}`) + throw new Error(`Failed to read "${this.fsPath}": ${error.message || error}`) + } + } + + private async readFile(filePath: string): Promise { + this.logging.info(`Reading file: ${filePath}`) + return await this.workspace.fs.readFile(this.fsPath) + } + + private handleFileRange(fullText: string): InvokeOutput { + if (!this.readRange || this.readRange.length === 0) { + this.logging.log('No range provided. returning entire file.') + return this.createOutput(this.enforceMaxSize(fullText)) + } + + const lines = fullText.split('\n') + const [start, end] = this.parseLineRange(lines.length, this.readRange) + if (start > end) { + this.logging.error(`Invalid range: ${this.readRange.join('-')}`) + return this.createOutput('') + } + + this.logging.log(`Reading file: ${this.fsPath}, lines ${start + 1}-${end + 1}`) + const slice = lines.slice(start, end + 1).join('\n') + return this.createOutput(this.enforceMaxSize(slice)) + } + + private parseLineRange(lineCount: number, range: number[]): [number, number] { + const startIdx = range[0] + let endIdx = range.length >= 2 ? range[1] : undefined + + if (endIdx === undefined) { + endIdx = -1 + } + + const convert = (i: number): number => { + return i < 0 ? lineCount + i : i - 1 + } + + const finalStart = Math.max(0, Math.min(lineCount - 1, convert(startIdx))) + const finalEnd = Math.max(0, Math.min(lineCount - 1, convert(endIdx))) + return [finalStart, finalEnd] + } + + private enforceMaxSize(content: string): string { + const byteCount = Buffer.byteLength(content, 'utf8') + if (byteCount > maxToolResponseSize) { + throw new Error( + `This tool only supports reading ${maxToolResponseSize} bytes at a time. + You tried to read ${byteCount} bytes. Try executing with fewer lines specified.` + ) + } + return content + } + + private createOutput(content: string): InvokeOutput { + return { + output: { + kind: OutputKind.Text, + content: content, + }, + } + } +} diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.ts new file mode 100644 index 0000000000..a1369592dc --- /dev/null +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.ts @@ -0,0 +1,28 @@ +import * as path from 'path' + +export const maxToolResponseSize = 30720 // 30KB + +export enum OutputKind { + Text = 'text', + Json = 'json', +} + +export interface InvokeOutput { + output: { + kind: OutputKind + content: string + } +} + +export function sanitizePath(inputPath: string, homePath: string): string { + let sanitized = inputPath.trim() + + if (sanitized.startsWith('~')) { + sanitized = path.join(homePath, sanitized.slice(1)) + } + + if (!path.isAbsolute(sanitized)) { + sanitized = path.resolve(sanitized) + } + return sanitized +} From c7692fa45641a4444ce51361daf5086a5cc5f96b Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 31 Mar 2025 15:01:11 -0400 Subject: [PATCH 02/20] refactor: move testFolder to a general location --- core/aws-lsp-core/src/index.ts | 1 + core/aws-lsp-core/src/test/testFolder.ts | 54 +++++++++++++++++++ core/aws-lsp-core/src/util/path.ts | 13 +++++ .../agenticChat/tools/fsRead.test.ts | 42 +++------------ .../agenticChat/tools/fsRead.ts | 8 ++- .../agenticChat/tools/toolShared.ts | 15 ------ 6 files changed, 78 insertions(+), 55 deletions(-) create mode 100644 core/aws-lsp-core/src/test/testFolder.ts diff --git a/core/aws-lsp-core/src/index.ts b/core/aws-lsp-core/src/index.ts index 8964a2bab5..dc5e071d54 100644 --- a/core/aws-lsp-core/src/index.ts +++ b/core/aws-lsp-core/src/index.ts @@ -14,3 +14,4 @@ export * as textUtils from './util/text' export * as timeoutUtils from './util/timeoutUtils' export * from './util/awsError' export * from './base/index' +export * from './test/testFolder' diff --git a/core/aws-lsp-core/src/test/testFolder.ts b/core/aws-lsp-core/src/test/testFolder.ts new file mode 100644 index 0000000000..7aca57914e --- /dev/null +++ b/core/aws-lsp-core/src/test/testFolder.ts @@ -0,0 +1,54 @@ +import * as path from 'path' +import * as fs from 'fs/promises' +import * as os from 'os' + +/** + * Interface for working with temporary files. + * Simplified port of https://github.com/aws/aws-toolkit-vscode/blob/16477869525fb79f8dc82cb22e301aaea9c5e0c6/packages/core/src/test/testUtil.ts#L77 + * + * Proper usage requires adding the proper logic into the hooks. See example below: + * + * before(async () => { + * ... + * testFolder = await TestFolder.create() + * ... + * } + * + * afterEach(async () => { + * ... + * await testFolder.clear() + * ... + * }) + * + * after(async () => { + * ... + * await testFolder.delete() + * ... + * }) + */ +export class TestFolder { + private constructor(public readonly folderPath: string) {} + + async write(fileName: string, content: string): Promise { + const filePath = path.join(this.folderPath, fileName) + await fs.writeFile(filePath, content) + return filePath + } + + static async create() { + const tempDir = path.join(os.type() === 'Darwin' ? '/tmp' : os.tmpdir(), 'aws-language-servers') + await fs.mkdir(tempDir, { recursive: true }) + return new TestFolder(tempDir) + } + + async delete() { + fs.rm(this.folderPath, { recursive: true, force: true }) + } + + async clear() { + const files = await fs.readdir(this.folderPath) + for (const f of files) { + await fs.rm(path.join(this.folderPath, f), { recursive: true, force: true }) + } + } +} diff --git a/core/aws-lsp-core/src/util/path.ts b/core/aws-lsp-core/src/util/path.ts index a65d1d5703..484f3e5716 100644 --- a/core/aws-lsp-core/src/util/path.ts +++ b/core/aws-lsp-core/src/util/path.ts @@ -78,3 +78,16 @@ export function normalize(p: string): string { } return normalizeSeparator(path.normalize(p)) } + +export function sanitize(inputPath: string): string { + let sanitized = inputPath.trim() + + if (sanitized.startsWith('~')) { + sanitized = path.join(os.homedir(), sanitized.slice(1)) + } + + if (!path.isAbsolute(sanitized)) { + sanitized = path.resolve(sanitized) + } + return sanitized +} diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts index 1c6c5b4e4e..14f971d834 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts @@ -2,10 +2,10 @@ import * as assert from 'assert' import { FsRead } from './fsRead' import * as path from 'path' import * as fs from 'fs/promises' -import * as os from 'os' import { TestFeatures } from '@aws/language-server-runtimes/testing' import { Workspace } from '@aws/language-server-runtimes/server-interface' import { StubbedInstance } from 'ts-sinon' +import { TestFolder } from '@aws/lsp-core' describe('FsRead Tool', () => { let features: TestFeatures @@ -38,7 +38,7 @@ describe('FsRead Tool', () => { }) it('throws if path is empty', async () => { - const fsRead = new FsRead(features, { path: '', homePath: os.homedir() }) + const fsRead = new FsRead(features, { path: '' }) await assert.rejects(fsRead.validate(), /Path cannot be empty/i, 'Expected an error about empty path') }) @@ -46,7 +46,7 @@ describe('FsRead Tool', () => { const fileContent = 'Line 1\nLine 2\nLine 3' const filePath = await testFolder.write('fullFile.txt', fileContent) - const fsRead = new FsRead(features, { path: filePath, homePath: os.homedir() }) + const fsRead = new FsRead(features, { path: filePath }) await fsRead.validate() const result = await fsRead.invoke(process.stdout) @@ -58,7 +58,7 @@ describe('FsRead Tool', () => { const fileContent = 'A\nB\nC\nD\nE\nF' const filePath = await testFolder.write('partialFile.txt', fileContent) - const fsRead = new FsRead(features, { path: filePath, readRange: [2, 4], homePath: os.homedir() }) + const fsRead = new FsRead(features, { path: filePath, readRange: [2, 4] }) await fsRead.validate() const result = await fsRead.invoke(process.stdout) @@ -68,7 +68,7 @@ describe('FsRead Tool', () => { it('throws error if path does not exist', async () => { const filePath = path.join(testFolder.folderPath, 'no_such_file.txt') - const fsRead = new FsRead(features, { path: filePath, homePath: os.homedir() }) + const fsRead = new FsRead(features, { path: filePath }) await assert.rejects( fsRead.validate(), @@ -82,7 +82,7 @@ describe('FsRead Tool', () => { const filePath = await testFolder.write('bigFile.txt', bigContent) - const fsRead = new FsRead(features, { homePath: os.homedir(), path: filePath }) + const fsRead = new FsRead(features, { path: filePath }) await fsRead.validate() await assert.rejects( @@ -94,7 +94,7 @@ describe('FsRead Tool', () => { it('invalid line range', async () => { const filePath = await testFolder.write('rangeTest.txt', '1\n2\n3') - const fsRead = new FsRead(features, { path: filePath, readRange: [3, 2], homePath: os.homedir() }) + const fsRead = new FsRead(features, { path: filePath, readRange: [3, 2] }) await fsRead.validate() const result = await fsRead.invoke(process.stdout) @@ -102,31 +102,3 @@ describe('FsRead Tool', () => { assert.strictEqual(result.output.content, '') }) }) - -// TODO: move this a test utility file. -class TestFolder { - private constructor(public readonly folderPath: string) {} - - async write(fileName: string, content: string): Promise { - const filePath = path.join(this.folderPath, fileName) - await fs.writeFile(filePath, content) - return filePath - } - - static async create() { - const tempDir = path.join(os.type() === 'Darwin' ? '/tmp' : os.tmpdir(), 'aws-language-servers') - await fs.mkdir(tempDir, { recursive: true }) - return new TestFolder(tempDir) - } - - async delete() { - fs.rm(this.folderPath, { recursive: true, force: true }) - } - - async clear() { - const files = await fs.readdir(this.folderPath) - for (const f of files) { - await fs.rm(path.join(this.folderPath, f), { recursive: true, force: true }) - } - } -} diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts index fcfbdf2c9a..da5bb890d1 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts @@ -2,27 +2,25 @@ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. * SPDX-License-Identifier: Apache-2.0 */ -import { InvokeOutput, maxToolResponseSize, OutputKind, sanitizePath } from './toolShared' +import { InvokeOutput, maxToolResponseSize, OutputKind } from './toolShared' import { Writable } from 'stream' import * as path from 'path' import { Features } from '@aws/language-server-runtimes/server-interface/server' +import { sanitize } from '@aws/lsp-core/out/util/path' export interface FsReadParams { path: string - homePath: string readRange?: number[] } export class FsRead { private fsPath: string - private readonly homePath: string private readonly readRange?: number[] private readonly logging: Features['logging'] private readonly workspace: Features['workspace'] constructor(features: Pick, params: FsReadParams) { this.fsPath = params.path - this.homePath = params.homePath this.readRange = params.readRange this.logging = features.logging this.workspace = features.workspace @@ -34,7 +32,7 @@ export class FsRead { throw new Error('Path cannot be empty.') } - const sanitized = sanitizePath(this.fsPath, this.homePath) + const sanitized = sanitize(this.fsPath) this.fsPath = sanitized let fileExists: boolean diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.ts index a1369592dc..24b564ea89 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.ts @@ -1,5 +1,3 @@ -import * as path from 'path' - export const maxToolResponseSize = 30720 // 30KB export enum OutputKind { @@ -13,16 +11,3 @@ export interface InvokeOutput { content: string } } - -export function sanitizePath(inputPath: string, homePath: string): string { - let sanitized = inputPath.trim() - - if (sanitized.startsWith('~')) { - sanitized = path.join(homePath, sanitized.slice(1)) - } - - if (!path.isAbsolute(sanitized)) { - sanitized = path.resolve(sanitized) - } - return sanitized -} From f70f694ab8f0cc84b06233e5a0b3b59dfa401aa5 Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 31 Mar 2025 15:35:58 -0400 Subject: [PATCH 03/20] feat: port over fsWrite with minimal changes --- .../agenticChat/tools/fsWrite.test.ts | 429 ++++++++++++++++++ .../agenticChat/tools/fsWrite.ts | 177 ++++++++ 2 files changed, 606 insertions(+) create mode 100644 server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts create mode 100644 server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts new file mode 100644 index 0000000000..f427a151b4 --- /dev/null +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts @@ -0,0 +1,429 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ +import { AppendParams, CreateParams, FsWrite, InsertParams, StrReplaceParams } from './fsWrite' +import { TestFolder } from '@aws/lsp-core' +import * as path from 'path' +import * as assert from 'assert' +import * as fs from 'fs/promises' +import { InvokeOutput, OutputKind } from './toolShared' +import { TestFeatures } from '@aws/language-server-runtimes/testing' +import { Workspace } from '@aws/language-server-runtimes/server-interface' +import { StubbedInstance } from 'ts-sinon' + +describe('FsWrite Tool', function () { + let testFolder: TestFolder + let features: TestFeatures + const expectedOutput: InvokeOutput = { + output: { + kind: OutputKind.Text, + content: '', + }, + } + + before(async function () { + features = new TestFeatures() + features.workspace = { + // @ts-ignore writing a file does not require all fs features implemented + fs: { + writeFile: fs.writeFile, + readFile: (path, options?) => + fs.readFile(path, { encoding: (options?.encoding || 'utf-8') as BufferEncoding }), + exists: path => + fs + .access(path) + .then(() => true) + .catch(() => false), + } as Workspace['fs'], + } as StubbedInstance + testFolder = await TestFolder.create() + }) + + after(async function () { + await testFolder.delete() + }) + + describe('handleCreate', function () { + it('creates a new file with fileText content', async function () { + const filePath = path.join(testFolder.path, 'file1.txt') + const fileExists = await features.workspace.fs.exists(filePath) + assert.ok(!fileExists) + + const params: CreateParams = { + command: 'create', + fileText: 'Hello World', + path: filePath, + } + const fsWrite = new FsWrite(features, params) + const output = await fsWrite.invoke(process.stdout) + + const content = await features.workspace.fs.readFile(filePath) + assert.strictEqual(content, 'Hello World') + + assert.deepStrictEqual(output, expectedOutput) + }) + + it('replaces existing file with fileText content', async function () { + const filePath = path.join(testFolder.path, 'file1.txt') + const fileExists = await features.workspace.fs.exists(filePath) + assert.ok(fileExists) + + const params: CreateParams = { + command: 'create', + fileText: 'Goodbye', + path: filePath, + } + const fsWrite = new FsWrite(features, params) + const output = await fsWrite.invoke(process.stdout) + + const content = await features.workspace.fs.readFile(filePath) + assert.strictEqual(content, 'Goodbye') + + assert.deepStrictEqual(output, expectedOutput) + }) + + it('uses newStr when fileText is not provided', async function () { + const filePath = path.join(testFolder.path, 'file2.txt') + + const params: CreateParams = { + command: 'create', + newStr: 'Hello World', + path: filePath, + } + const fsWrite = new FsWrite(features, params) + const output = await fsWrite.invoke(process.stdout) + + const content = await features.workspace.fs.readFile(filePath) + assert.strictEqual(content, 'Hello World') + + assert.deepStrictEqual(output, expectedOutput) + }) + + it('creates an empty file when no content is provided', async function () { + const filePath = path.join(testFolder.path, 'file3.txt') + + const params: CreateParams = { + command: 'create', + path: filePath, + } + const fsWrite = new FsWrite(features, params) + const output = await fsWrite.invoke(process.stdout) + + const content = await features.workspace.fs.readFile(filePath) + assert.strictEqual(content, '') + + assert.deepStrictEqual(output, expectedOutput) + }) + }) + + describe('handleStrReplace', async function () { + before(async function () { + testFolder = await TestFolder.create() + }) + + it('replaces a single occurrence of a string', async function () { + const filePath = path.join(testFolder.path, 'file1.txt') + await fs.writeFile(filePath, 'Hello World') + + const params: StrReplaceParams = { + command: 'strReplace', + path: filePath, + oldStr: 'Hello', + newStr: 'Goodbye', + } + const fsWrite = new FsWrite(features, params) + const output = await fsWrite.invoke(process.stdout) + + const content = await features.workspace.fs.readFile(filePath) + assert.strictEqual(content, 'Goodbye World') + + assert.deepStrictEqual(output, expectedOutput) + }) + + it('throws error when no matches are found', async function () { + const filePath = path.join(testFolder.path, 'file1.txt') + + const params: StrReplaceParams = { + command: 'strReplace', + path: filePath, + oldStr: 'Invalid', + newStr: 'Goodbye', + } + + const fsWrite = new FsWrite(features, params) + await assert.rejects(() => fsWrite.invoke(process.stdout), /No occurrences of "Invalid" were found/) + }) + + it('throws error when multiple matches are found', async function () { + const filePath = path.join(testFolder.path, 'file2.txt') + await fs.writeFile(filePath, 'Hello Hello World') + + const params: StrReplaceParams = { + command: 'strReplace', + path: filePath, + oldStr: 'Hello', + newStr: 'Goodbye', + } + + const fsWrite = new FsWrite(features, params) + await assert.rejects( + () => fsWrite.invoke(process.stdout), + /2 occurrences of oldStr were found when only 1 is expected/ + ) + }) + + it('handles regular expression special characters correctly', async function () { + const filePath = path.join(testFolder.path, 'file3.txt') + await fs.writeFile(filePath, 'Text with special chars: .*+?^${}()|[]\\') + + const params: StrReplaceParams = { + command: 'strReplace', + path: filePath, + oldStr: '.*+?^${}()|[]\\', + newStr: 'REPLACED', + } + const fsWrite = new FsWrite(features, params) + const output = await fsWrite.invoke(process.stdout) + + const content = await features.workspace.fs.readFile(filePath) + assert.strictEqual(content, 'Text with special chars: REPLACED') + + assert.deepStrictEqual(output, expectedOutput) + }) + + it('preserves whitespace and newlines during replacement', async function () { + const filePath = path.join(testFolder.path, 'file4.txt') + await fs.writeFile(filePath, 'Line 1\n Indented line\nLine 3') + + const params: StrReplaceParams = { + command: 'strReplace', + path: filePath, + oldStr: ' Indented line\n', + newStr: ' Double indented\n', + } + const fsWrite = new FsWrite(features, params) + const output = await fsWrite.invoke(process.stdout) + + const content = await features.workspace.fs.readFile(filePath) + assert.strictEqual(content, 'Line 1\n Double indented\nLine 3') + + assert.deepStrictEqual(output, expectedOutput) + }) + }) + + describe('handleInsert', function () { + before(async function () { + testFolder = await TestFolder.create() + }) + + it('inserts text after the specified line number', async function () { + const filePath = path.join(testFolder.path, 'file1.txt') + await fs.writeFile(filePath, 'Line 1\nLine 2\nLine 3\nLine 4') + + const params: InsertParams = { + command: 'insert', + path: filePath, + insertLine: 2, + newStr: 'New Line', + } + const fsWrite = new FsWrite(features, params) + const output = await fsWrite.invoke(process.stdout) + + const newContent = await features.workspace.fs.readFile(filePath) + assert.strictEqual(newContent, 'Line 1\nLine 2\nNew Line\nLine 3\nLine 4') + + assert.deepStrictEqual(output, expectedOutput) + }) + + it('inserts text at the beginning when line number is 0', async function () { + const filePath = path.join(testFolder.path, 'file1.txt') + const params: InsertParams = { + command: 'insert', + path: filePath, + insertLine: 0, + newStr: 'New First Line', + } + const fsWrite = new FsWrite(features, params) + const output = await fsWrite.invoke(process.stdout) + + const newContent = await features.workspace.fs.readFile(filePath) + assert.strictEqual(newContent, 'New First Line\nLine 1\nLine 2\nNew Line\nLine 3\nLine 4') + + assert.deepStrictEqual(output, expectedOutput) + }) + + it('inserts text at the end when line number exceeds file length', async function () { + const filePath = path.join(testFolder.path, 'file1.txt') + const params: InsertParams = { + command: 'insert', + path: filePath, + insertLine: 10, + newStr: 'New Last Line', + } + const fsWrite = new FsWrite(features, params) + const output = await fsWrite.invoke(process.stdout) + + const newContent = await features.workspace.fs.readFile(filePath) + assert.strictEqual(newContent, 'New First Line\nLine 1\nLine 2\nNew Line\nLine 3\nLine 4\nNew Last Line') + + assert.deepStrictEqual(output, expectedOutput) + }) + + it('handles insertion into an empty file', async function () { + const filePath = path.join(testFolder.path, 'file2.txt') + await fs.writeFile(filePath, '') + + const params: InsertParams = { + command: 'insert', + path: filePath, + insertLine: 0, + newStr: 'First Line', + } + const fsWrite = new FsWrite(features, params) + const output = await fsWrite.invoke(process.stdout) + + const newContent = await features.workspace.fs.readFile(filePath) + assert.strictEqual(newContent, 'First Line\n') + + assert.deepStrictEqual(output, expectedOutput) + }) + + it('handles negative line numbers by inserting at the beginning', async function () { + const filePath = path.join(testFolder.path, 'file2.txt') + + const params: InsertParams = { + command: 'insert', + path: filePath, + insertLine: -1, + newStr: 'New First Line', + } + const fsWrite = new FsWrite(features, params) + const output = await fsWrite.invoke(process.stdout) + + const newContent = await features.workspace.fs.readFile(filePath) + assert.strictEqual(newContent, 'New First Line\nFirst Line\n') + + assert.deepStrictEqual(output, expectedOutput) + }) + + it('throws error when file does not exist', async function () { + const filePath = path.join(testFolder.path, 'nonexistent.txt') + + const params: InsertParams = { + command: 'insert', + path: filePath, + insertLine: 1, + newStr: 'New Line', + } + + const fsWrite = new FsWrite(features, params) + await assert.rejects(() => fsWrite.invoke(process.stdout), /no such file or directory/) + }) + }) + + describe('handleAppend', function () { + it('appends text to the end of a file', async function () { + const filePath = path.join(testFolder.path, 'file1.txt') + await fs.writeFile(filePath, 'Line 1\nLine 2\nLine 3\n') + + const params: AppendParams = { + command: 'append', + path: filePath, + newStr: 'Line 4', + } + + const fsWrite = new FsWrite(features, params) + const output = await fsWrite.invoke(process.stdout) + + const newContent = await features.workspace.fs.readFile(filePath) + assert.strictEqual(newContent, 'Line 1\nLine 2\nLine 3\nLine 4') + + assert.deepStrictEqual(output, expectedOutput) + }) + + it('adds a newline before appending if file does not end with one', async function () { + const filePath = path.join(testFolder.path, 'file2.txt') + await fs.writeFile(filePath, 'Line 1\nLine 2\nLine 3') + + const params: AppendParams = { + command: 'append', + path: filePath, + newStr: 'Line 4', + } + + const fsWrite = new FsWrite(features, params) + const output = await fsWrite.invoke(process.stdout) + + const newContent = await features.workspace.fs.readFile(filePath) + assert.strictEqual(newContent, 'Line 1\nLine 2\nLine 3\nLine 4') + + assert.deepStrictEqual(output, expectedOutput) + }) + + it('appends to an empty file', async function () { + const filePath = path.join(testFolder.path, 'file3.txt') + await fs.writeFile(filePath, '') + + const params: AppendParams = { + command: 'append', + path: filePath, + newStr: 'Line 1', + } + const fsWrite = new FsWrite(features, params) + const output = await fsWrite.invoke(process.stdout) + + const newContent = await features.workspace.fs.readFile(filePath) + assert.strictEqual(newContent, 'Line 1') + + assert.deepStrictEqual(output, expectedOutput) + }) + + it('appends multiple lines correctly', async function () { + const filePath = path.join(testFolder.path, 'file3.txt') + + const params: AppendParams = { + command: 'append', + path: filePath, + newStr: 'Line 2\nLine 3', + } + const fsWrite = new FsWrite(features, params) + const output = await fsWrite.invoke(process.stdout) + + const newContent = await features.workspace.fs.readFile(filePath) + assert.strictEqual(newContent, 'Line 1\nLine 2\nLine 3') + + assert.deepStrictEqual(output, expectedOutput) + }) + + it('handles appending empty string', async function () { + const filePath = path.join(testFolder.path, 'file3.txt') + + const params: AppendParams = { + command: 'append', + path: filePath, + newStr: '', + } + const fsWrite = new FsWrite(features, params) + const output = await fsWrite.invoke(process.stdout) + + const newContent = await features.workspace.fs.readFile(filePath) + assert.strictEqual(newContent, 'Line 1\nLine 2\nLine 3\n') + + assert.deepStrictEqual(output, expectedOutput) + }) + + it('throws error when file does not exist', async function () { + const filePath = path.join(testFolder.path, 'nonexistent.txt') + + const params: AppendParams = { + command: 'append', + path: filePath, + newStr: 'New Line', + } + + const fsWrite = new FsWrite(features, params) + await assert.rejects(() => fsWrite.invoke(process.stdout), /no such file or directory/) + }) + }) +}) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts new file mode 100644 index 0000000000..c897e59863 --- /dev/null +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts @@ -0,0 +1,177 @@ +import { InvokeOutput, OutputKind } from './toolShared' +import { Writable } from 'stream' +import * as path from 'path' +import { Features } from '@aws/language-server-runtimes/server-interface/server' +import { sanitize } from '@aws/lsp-core/out/util/path' + +interface BaseParams { + path: string +} + +export interface CreateParams extends BaseParams { + command: 'create' + fileText?: string + newStr?: string +} + +export interface StrReplaceParams extends BaseParams { + command: 'strReplace' + oldStr: string + newStr: string +} + +export interface InsertParams extends BaseParams { + command: 'insert' + insertLine: number + newStr: string +} + +export interface AppendParams extends BaseParams { + command: 'append' + newStr: string +} + +export type FsWriteParams = CreateParams | StrReplaceParams | InsertParams | AppendParams + +export class FsWrite { + private fsPath: string + private readonly logging: Features['logging'] + private readonly workspace: Features['workspace'] + + constructor( + features: Pick, + readonly params: FsWriteParams + ) { + this.fsPath = params.path + this.logging = features.logging + this.workspace = features.workspace + } + + public async invoke(_updates: Writable): Promise { + const sanitizedPath = sanitize(this.params.path) + + switch (this.params.command) { + case 'create': + await this.handleCreate(this.params, sanitizedPath) + break + case 'strReplace': + await this.handleStrReplace(this.params, sanitizedPath) + break + case 'insert': + await this.handleInsert(this.params, sanitizedPath) + break + case 'append': + await this.handleAppend(this.params, sanitizedPath) + break + } + + return { + output: { + kind: OutputKind.Text, + content: '', + }, + } + } + + public queueDescription(updates: Writable): void { + const fileName = path.basename(this.fsPath) + updates.write(`Writing to: [${fileName}](${this.params.path})`) + updates.end() + } + + public async validate(): Promise { + switch (this.params.command) { + case 'create': + if (!this.params.path) { + throw new Error('Path must not be empty') + } + break + case 'strReplace': + case 'insert': { + const fileExists = await this.workspace.fs.exists(this.params.path) + if (!fileExists) { + throw new Error('The provided path must exist in order to replace or insert contents into it') + } + break + } + case 'append': + if (!this.params.path) { + throw new Error('Path must not be empty') + } + if (!this.params.newStr) { + throw new Error('Content to append must not be empty') + } + break + } + } + + private async handleCreate(params: CreateParams, sanitizedPath: string): Promise { + const content = this.getCreateCommandText(params) + + const fileExists = await this.workspace.fs.exists(sanitizedPath) + + await this.workspace.fs.writeFile(sanitizedPath, content) + } + + private async handleStrReplace(params: StrReplaceParams, sanitizedPath: string): Promise { + const fileContent = await this.workspace.fs.readFile(sanitizedPath) + + const matches = [...fileContent.matchAll(new RegExp(this.escapeRegExp(params.oldStr), 'g'))] + + if (matches.length === 0) { + throw new Error(`No occurrences of "${params.oldStr}" were found`) + } + if (matches.length > 1) { + throw new Error(`${matches.length} occurrences of oldStr were found when only 1 is expected`) + } + + const newContent = fileContent.replace(params.oldStr, params.newStr) + await this.workspace.fs.writeFile(sanitizedPath, newContent) + } + + private async handleInsert(params: InsertParams, sanitizedPath: string): Promise { + const fileContent = await this.workspace.fs.readFile(sanitizedPath) + const lines = fileContent.split('\n') + + const numLines = lines.length + const insertLine = Math.max(0, Math.min(params.insertLine, numLines)) + + let newContent: string + if (insertLine === 0) { + newContent = params.newStr + '\n' + fileContent + } else { + newContent = [...lines.slice(0, insertLine), params.newStr, ...lines.slice(insertLine)].join('\n') + } + + await this.workspace.fs.writeFile(sanitizedPath, newContent) + } + + private async handleAppend(params: AppendParams, sanitizedPath: string): Promise { + const fileContent = await this.workspace.fs.readFile(sanitizedPath) + const needsNewline = fileContent.length !== 0 && !fileContent.endsWith('\n') + + let contentToAppend = params.newStr + if (needsNewline) { + contentToAppend = '\n' + contentToAppend + } + + const newContent = fileContent + contentToAppend + await this.workspace.fs.writeFile(sanitizedPath, newContent) + } + + private getCreateCommandText(params: CreateParams): string { + if (params.fileText) { + return params.fileText + } + if (params.newStr) { + this.logging.warn('Required field `fileText` is missing, use the provided `newStr` instead') + return params.newStr + } + this.logging.warn('No content provided for the create command') + return '' + } + + private escapeRegExp(string: string): string { + return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + } +} From 0d8f7b923c1afdd5828c8a9fa599335b13cfadd4 Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 31 Mar 2025 15:41:42 -0400 Subject: [PATCH 04/20] refactor: clean up testFolder util --- core/aws-lsp-core/src/test/testFolder.ts | 19 ++++++++++++------- .../agenticChat/tools/fsRead.test.ts | 2 +- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/core/aws-lsp-core/src/test/testFolder.ts b/core/aws-lsp-core/src/test/testFolder.ts index 7aca57914e..4bfc99dca3 100644 --- a/core/aws-lsp-core/src/test/testFolder.ts +++ b/core/aws-lsp-core/src/test/testFolder.ts @@ -13,7 +13,7 @@ import * as os from 'os' * testFolder = await TestFolder.create() * ... * } - * + * // Only necessary if test state should not bleed through. * afterEach(async () => { * ... * await testFolder.clear() @@ -27,28 +27,33 @@ import * as os from 'os' * }) */ export class TestFolder { - private constructor(public readonly folderPath: string) {} + private constructor(public readonly path: string) {} async write(fileName: string, content: string): Promise { - const filePath = path.join(this.folderPath, fileName) + const filePath = path.join(this.path, fileName) await fs.writeFile(filePath, content) return filePath } static async create() { - const tempDir = path.join(os.type() === 'Darwin' ? '/tmp' : os.tmpdir(), 'aws-language-servers') + const tempDir = path.join( + os.type() === 'Darwin' ? '/tmp' : os.tmpdir(), + 'aws-language-servers', + 'test', + crypto.randomUUID() + ) await fs.mkdir(tempDir, { recursive: true }) return new TestFolder(tempDir) } async delete() { - fs.rm(this.folderPath, { recursive: true, force: true }) + fs.rm(this.path, { recursive: true, force: true }) } async clear() { - const files = await fs.readdir(this.folderPath) + const files = await fs.readdir(this.path) for (const f of files) { - await fs.rm(path.join(this.folderPath, f), { recursive: true, force: true }) + await fs.rm(path.join(this.path, f), { recursive: true, force: true }) } } } diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts index 14f971d834..c9204569ef 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts @@ -67,7 +67,7 @@ describe('FsRead Tool', () => { }) it('throws error if path does not exist', async () => { - const filePath = path.join(testFolder.folderPath, 'no_such_file.txt') + const filePath = path.join(testFolder.path, 'no_such_file.txt') const fsRead = new FsRead(features, { path: filePath }) await assert.rejects( From 706747b46a0138823842664a460ea538c4ff20cd Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 31 Mar 2025 15:44:43 -0400 Subject: [PATCH 05/20] docs: add links to original implementations --- .../src/language-server/agenticChat/tools/fsRead.ts | 2 ++ .../src/language-server/agenticChat/tools/fsWrite.ts | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts index da5bb890d1..ca6bb33da7 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts @@ -8,6 +8,8 @@ import * as path from 'path' import { Features } from '@aws/language-server-runtimes/server-interface/server' import { sanitize } from '@aws/lsp-core/out/util/path' +// Port of https://github.com/aws/aws-toolkit-vscode/blob/10bb1c7dc45f128df14d749d95905c0e9b808096/packages/core/src/codewhispererChat/tools/fsRead.ts#L17 + export interface FsReadParams { path: string readRange?: number[] diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts index c897e59863..553b941230 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts @@ -4,6 +4,8 @@ import * as path from 'path' import { Features } from '@aws/language-server-runtimes/server-interface/server' import { sanitize } from '@aws/lsp-core/out/util/path' +// Port of https://github.com/aws/aws-toolkit-vscode/blob/10bb1c7dc45f128df14d749d95905c0e9b808096/packages/core/src/codewhispererChat/tools/fsWrite.ts#L42 + interface BaseParams { path: string } @@ -108,8 +110,6 @@ export class FsWrite { private async handleCreate(params: CreateParams, sanitizedPath: string): Promise { const content = this.getCreateCommandText(params) - const fileExists = await this.workspace.fs.exists(sanitizedPath) - await this.workspace.fs.writeFile(sanitizedPath, content) } From f441d4de1be9e83746383599cc69f70088d610f1 Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 31 Mar 2025 16:07:01 -0400 Subject: [PATCH 06/20] refactor: clean up comments --- .../src/language-server/agenticChat/tools/fsRead.test.ts | 2 +- .../src/language-server/agenticChat/tools/fsWrite.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts index c9204569ef..0ac0d9b5f4 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts @@ -14,7 +14,7 @@ describe('FsRead Tool', () => { before(async () => { features = new TestFeatures() features.workspace = { - // @ts-ignore reading a file should use readOnly fs operations + // @ts-ignore reading a file does not require all of fs to be implemented. fs: { readFile: (path, options?) => fs.readFile(path, { encoding: (options?.encoding || 'utf-8') as BufferEncoding }), diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts index f427a151b4..bb98375b20 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts @@ -25,7 +25,7 @@ describe('FsWrite Tool', function () { before(async function () { features = new TestFeatures() features.workspace = { - // @ts-ignore writing a file does not require all fs features implemented + // @ts-ignore writing a file does not require all of fs to be implemented fs: { writeFile: fs.writeFile, readFile: (path, options?) => From 8febafb50e24f70a667b16b82036480728b4d9ed Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 31 Mar 2025 16:20:26 -0400 Subject: [PATCH 07/20] fix: properly import crypto --- core/aws-lsp-core/src/test/testFolder.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/core/aws-lsp-core/src/test/testFolder.ts b/core/aws-lsp-core/src/test/testFolder.ts index 4bfc99dca3..29e7a0a459 100644 --- a/core/aws-lsp-core/src/test/testFolder.ts +++ b/core/aws-lsp-core/src/test/testFolder.ts @@ -1,6 +1,7 @@ import * as path from 'path' -import * as fs from 'fs/promises' +import * as fs from 'fs' import * as os from 'os' +import * as crypto from 'crypto' /** * Interface for working with temporary files. @@ -31,7 +32,7 @@ export class TestFolder { async write(fileName: string, content: string): Promise { const filePath = path.join(this.path, fileName) - await fs.writeFile(filePath, content) + fs.writeFileSync(filePath, content) return filePath } @@ -40,20 +41,20 @@ export class TestFolder { os.type() === 'Darwin' ? '/tmp' : os.tmpdir(), 'aws-language-servers', 'test', - crypto.randomUUID() + crypto.randomBytes(4).toString('hex') ) - await fs.mkdir(tempDir, { recursive: true }) + await fs.promises.mkdir(tempDir, { recursive: true }) return new TestFolder(tempDir) } async delete() { - fs.rm(this.path, { recursive: true, force: true }) + fs.rmSync(this.path, { recursive: true, force: true }) } async clear() { - const files = await fs.readdir(this.path) + const files = await fs.readdirSync(this.path) for (const f of files) { - await fs.rm(path.join(this.path, f), { recursive: true, force: true }) + await fs.rmSync(path.join(this.path, f), { recursive: true, force: true }) } } } From 108f8d47a4ac2d9b92aa27fc3cdc8f2e34fb5f63 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 1 Apr 2025 09:57:40 -0400 Subject: [PATCH 08/20] refactor: avoid explicit scope down of features --- .../src/language-server/agenticChat/tools/fsRead.ts | 2 +- .../src/language-server/agenticChat/tools/fsWrite.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts index ca6bb33da7..b7b4361bed 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts @@ -21,7 +21,7 @@ export class FsRead { private readonly logging: Features['logging'] private readonly workspace: Features['workspace'] - constructor(features: Pick, params: FsReadParams) { + constructor(features: Pick & Partial, params: FsReadParams) { this.fsPath = params.path this.readRange = params.readRange this.logging = features.logging diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts index 553b941230..65355f64b5 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts @@ -41,7 +41,7 @@ export class FsWrite { private readonly workspace: Features['workspace'] constructor( - features: Pick, + features: Pick & Partial, readonly params: FsWriteParams ) { this.fsPath = params.path From 9f72fc5a9216c95a316086b700bfd2af1f4fc6b1 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 1 Apr 2025 10:24:57 -0400 Subject: [PATCH 09/20] fix: lint rules --- .../agenticChat/tools/fsRead.test.ts | 14 ++++-- .../agenticChat/tools/fsRead.ts | 20 ++++---- .../agenticChat/tools/fsWrite.test.ts | 48 +++++++++++-------- .../agenticChat/tools/fsWrite.ts | 12 ++--- 4 files changed, 51 insertions(+), 43 deletions(-) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts index 0ac0d9b5f4..7592f851e0 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts @@ -11,6 +11,12 @@ describe('FsRead Tool', () => { let features: TestFeatures let testFolder: TestFolder + const stdout = new WritableStream({ + write(chunk) { + process.stdout.write(chunk) + }, + }) + before(async () => { features = new TestFeatures() features.workspace = { @@ -48,7 +54,7 @@ describe('FsRead Tool', () => { const fsRead = new FsRead(features, { path: filePath }) await fsRead.validate() - const result = await fsRead.invoke(process.stdout) + const result = await fsRead.invoke(stdout) assert.strictEqual(result.output.kind, 'text', 'Output kind should be "text"') assert.strictEqual(result.output.content, fileContent, 'File content should match exactly') @@ -60,7 +66,7 @@ describe('FsRead Tool', () => { const fsRead = new FsRead(features, { path: filePath, readRange: [2, 4] }) await fsRead.validate() - const result = await fsRead.invoke(process.stdout) + const result = await fsRead.invoke(stdout) assert.strictEqual(result.output.kind, 'text') assert.strictEqual(result.output.content, 'B\nC\nD') @@ -86,7 +92,7 @@ describe('FsRead Tool', () => { await fsRead.validate() await assert.rejects( - fsRead.invoke(process.stdout), + fsRead.invoke(stdout), /This tool only supports reading \d+ bytes at a time/i, 'Expected a size-limit error' ) @@ -97,7 +103,7 @@ describe('FsRead Tool', () => { const fsRead = new FsRead(features, { path: filePath, readRange: [3, 2] }) await fsRead.validate() - const result = await fsRead.invoke(process.stdout) + const result = await fsRead.invoke(stdout) assert.strictEqual(result.output.kind, 'text') assert.strictEqual(result.output.content, '') }) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts index b7b4361bed..2cd81d4a91 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts @@ -3,8 +3,6 @@ * SPDX-License-Identifier: Apache-2.0 */ import { InvokeOutput, maxToolResponseSize, OutputKind } from './toolShared' -import { Writable } from 'stream' -import * as path from 'path' import { Features } from '@aws/language-server-runtimes/server-interface/server' import { sanitize } from '@aws/lsp-core/out/util/path' @@ -50,27 +48,27 @@ export class FsRead { this.logging.log(`Validation succeeded for path: ${this.fsPath}`) } - public queueDescription(updates: Writable): void { - const fileName = path.basename(this.fsPath) - updates.write(`Reading file: [${fileName}](${this.fsPath}), `) + public async queueDescription(updates: WritableStream): Promise { + const writer = updates.getWriter() + await writer.write(`Reading file: (${this.fsPath}), `) const [start, end] = this.readRange ?? [] if (start && end) { - updates.write(`from line ${start} to ${end}`) + await writer.write(`from line ${start} to ${end}`) } else if (start) { if (start > 0) { - updates.write(`from line ${start} to end of file`) + await writer.write(`from line ${start} to end of file`) } else { - updates.write(`${start} line from the end of file to end of file`) + await writer.write(`${start} line from the end of file to end of file`) } } else { - updates.write('all lines') + await writer.write('all lines') } - updates.end() + await writer.close() } - public async invoke(_updates: Writable): Promise { + public async invoke(_updates: WritableStream): Promise { try { const fileContents = await this.readFile(this.fsPath) this.logging.info(`Read file: ${this.fsPath}, size: ${fileContents.length}`) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts index bb98375b20..4f2874a180 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts @@ -22,6 +22,12 @@ describe('FsWrite Tool', function () { }, } + const stdout = new WritableStream({ + write(chunk) { + process.stdout.write(chunk) + }, + }) + before(async function () { features = new TestFeatures() features.workspace = { @@ -56,7 +62,7 @@ describe('FsWrite Tool', function () { path: filePath, } const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(process.stdout) + const output = await fsWrite.invoke(stdout) const content = await features.workspace.fs.readFile(filePath) assert.strictEqual(content, 'Hello World') @@ -75,7 +81,7 @@ describe('FsWrite Tool', function () { path: filePath, } const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(process.stdout) + const output = await fsWrite.invoke(stdout) const content = await features.workspace.fs.readFile(filePath) assert.strictEqual(content, 'Goodbye') @@ -92,7 +98,7 @@ describe('FsWrite Tool', function () { path: filePath, } const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(process.stdout) + const output = await fsWrite.invoke(stdout) const content = await features.workspace.fs.readFile(filePath) assert.strictEqual(content, 'Hello World') @@ -108,7 +114,7 @@ describe('FsWrite Tool', function () { path: filePath, } const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(process.stdout) + const output = await fsWrite.invoke(stdout) const content = await features.workspace.fs.readFile(filePath) assert.strictEqual(content, '') @@ -133,7 +139,7 @@ describe('FsWrite Tool', function () { newStr: 'Goodbye', } const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(process.stdout) + const output = await fsWrite.invoke(stdout) const content = await features.workspace.fs.readFile(filePath) assert.strictEqual(content, 'Goodbye World') @@ -152,7 +158,7 @@ describe('FsWrite Tool', function () { } const fsWrite = new FsWrite(features, params) - await assert.rejects(() => fsWrite.invoke(process.stdout), /No occurrences of "Invalid" were found/) + await assert.rejects(() => fsWrite.invoke(stdout), /No occurrences of "Invalid" were found/) }) it('throws error when multiple matches are found', async function () { @@ -168,7 +174,7 @@ describe('FsWrite Tool', function () { const fsWrite = new FsWrite(features, params) await assert.rejects( - () => fsWrite.invoke(process.stdout), + () => fsWrite.invoke(stdout), /2 occurrences of oldStr were found when only 1 is expected/ ) }) @@ -184,7 +190,7 @@ describe('FsWrite Tool', function () { newStr: 'REPLACED', } const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(process.stdout) + const output = await fsWrite.invoke(stdout) const content = await features.workspace.fs.readFile(filePath) assert.strictEqual(content, 'Text with special chars: REPLACED') @@ -203,7 +209,7 @@ describe('FsWrite Tool', function () { newStr: ' Double indented\n', } const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(process.stdout) + const output = await fsWrite.invoke(stdout) const content = await features.workspace.fs.readFile(filePath) assert.strictEqual(content, 'Line 1\n Double indented\nLine 3') @@ -228,7 +234,7 @@ describe('FsWrite Tool', function () { newStr: 'New Line', } const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(process.stdout) + const output = await fsWrite.invoke(stdout) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'Line 1\nLine 2\nNew Line\nLine 3\nLine 4') @@ -245,7 +251,7 @@ describe('FsWrite Tool', function () { newStr: 'New First Line', } const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(process.stdout) + const output = await fsWrite.invoke(stdout) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'New First Line\nLine 1\nLine 2\nNew Line\nLine 3\nLine 4') @@ -262,7 +268,7 @@ describe('FsWrite Tool', function () { newStr: 'New Last Line', } const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(process.stdout) + const output = await fsWrite.invoke(stdout) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'New First Line\nLine 1\nLine 2\nNew Line\nLine 3\nLine 4\nNew Last Line') @@ -281,7 +287,7 @@ describe('FsWrite Tool', function () { newStr: 'First Line', } const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(process.stdout) + const output = await fsWrite.invoke(stdout) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'First Line\n') @@ -299,7 +305,7 @@ describe('FsWrite Tool', function () { newStr: 'New First Line', } const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(process.stdout) + const output = await fsWrite.invoke(stdout) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'New First Line\nFirst Line\n') @@ -318,7 +324,7 @@ describe('FsWrite Tool', function () { } const fsWrite = new FsWrite(features, params) - await assert.rejects(() => fsWrite.invoke(process.stdout), /no such file or directory/) + await assert.rejects(() => fsWrite.invoke(stdout), /no such file or directory/) }) }) @@ -334,7 +340,7 @@ describe('FsWrite Tool', function () { } const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(process.stdout) + const output = await fsWrite.invoke(stdout) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'Line 1\nLine 2\nLine 3\nLine 4') @@ -353,7 +359,7 @@ describe('FsWrite Tool', function () { } const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(process.stdout) + const output = await fsWrite.invoke(stdout) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'Line 1\nLine 2\nLine 3\nLine 4') @@ -371,7 +377,7 @@ describe('FsWrite Tool', function () { newStr: 'Line 1', } const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(process.stdout) + const output = await fsWrite.invoke(stdout) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'Line 1') @@ -388,7 +394,7 @@ describe('FsWrite Tool', function () { newStr: 'Line 2\nLine 3', } const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(process.stdout) + const output = await fsWrite.invoke(stdout) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'Line 1\nLine 2\nLine 3') @@ -405,7 +411,7 @@ describe('FsWrite Tool', function () { newStr: '', } const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(process.stdout) + const output = await fsWrite.invoke(stdout) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'Line 1\nLine 2\nLine 3\n') @@ -423,7 +429,7 @@ describe('FsWrite Tool', function () { } const fsWrite = new FsWrite(features, params) - await assert.rejects(() => fsWrite.invoke(process.stdout), /no such file or directory/) + await assert.rejects(() => fsWrite.invoke(stdout), /no such file or directory/) }) }) }) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts index 65355f64b5..ad4f1a62bc 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts @@ -1,6 +1,4 @@ import { InvokeOutput, OutputKind } from './toolShared' -import { Writable } from 'stream' -import * as path from 'path' import { Features } from '@aws/language-server-runtimes/server-interface/server' import { sanitize } from '@aws/lsp-core/out/util/path' @@ -49,7 +47,7 @@ export class FsWrite { this.workspace = features.workspace } - public async invoke(_updates: Writable): Promise { + public async invoke(_updates: WritableStream): Promise { const sanitizedPath = sanitize(this.params.path) switch (this.params.command) { @@ -75,10 +73,10 @@ export class FsWrite { } } - public queueDescription(updates: Writable): void { - const fileName = path.basename(this.fsPath) - updates.write(`Writing to: [${fileName}](${this.params.path})`) - updates.end() + public async queueDescription(updates: WritableStream): Promise { + const writer = updates.getWriter() + await writer.write(`Writing to: (${this.params.path})`) + await writer.close() } public async validate(): Promise { From 49f47b1bfe87a6f38fff3b4fd538bb4e58f33515 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 1 Apr 2025 11:03:21 -0400 Subject: [PATCH 10/20] refactor: name export in index.ts --- core/aws-lsp-core/src/index.ts | 2 +- .../agenticChat/tools/fsRead.test.ts | 20 +++---- .../agenticChat/tools/fsWrite.test.ts | 54 +++++++++---------- 3 files changed, 38 insertions(+), 38 deletions(-) diff --git a/core/aws-lsp-core/src/index.ts b/core/aws-lsp-core/src/index.ts index dc5e071d54..1e739785a2 100644 --- a/core/aws-lsp-core/src/index.ts +++ b/core/aws-lsp-core/src/index.ts @@ -14,4 +14,4 @@ export * as textUtils from './util/text' export * as timeoutUtils from './util/timeoutUtils' export * from './util/awsError' export * from './base/index' -export * from './test/testFolder' +export * as testFolder from './test/testFolder' diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts index 7592f851e0..1f98c97107 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts @@ -4,12 +4,12 @@ import * as path from 'path' import * as fs from 'fs/promises' import { TestFeatures } from '@aws/language-server-runtimes/testing' import { Workspace } from '@aws/language-server-runtimes/server-interface' +import { testFolder } from '@aws/lsp-core' import { StubbedInstance } from 'ts-sinon' -import { TestFolder } from '@aws/lsp-core' describe('FsRead Tool', () => { let features: TestFeatures - let testFolder: TestFolder + let tempFolder: testFolder.TestFolder const stdout = new WritableStream({ write(chunk) { @@ -32,15 +32,15 @@ describe('FsRead Tool', () => { .catch(() => false), } as Workspace['fs'], } as StubbedInstance - testFolder = await TestFolder.create() + tempFolder = await testFolder.TestFolder.create() }) after(async () => { - testFolder.delete() + tempFolder.delete() }) afterEach(async () => { - testFolder.clear() + tempFolder.clear() }) it('throws if path is empty', async () => { @@ -50,7 +50,7 @@ describe('FsRead Tool', () => { it('reads entire file', async () => { const fileContent = 'Line 1\nLine 2\nLine 3' - const filePath = await testFolder.write('fullFile.txt', fileContent) + const filePath = await tempFolder.write('fullFile.txt', fileContent) const fsRead = new FsRead(features, { path: filePath }) await fsRead.validate() @@ -62,7 +62,7 @@ describe('FsRead Tool', () => { it('reads partial lines of a file', async () => { const fileContent = 'A\nB\nC\nD\nE\nF' - const filePath = await testFolder.write('partialFile.txt', fileContent) + const filePath = await tempFolder.write('partialFile.txt', fileContent) const fsRead = new FsRead(features, { path: filePath, readRange: [2, 4] }) await fsRead.validate() @@ -73,7 +73,7 @@ describe('FsRead Tool', () => { }) it('throws error if path does not exist', async () => { - const filePath = path.join(testFolder.path, 'no_such_file.txt') + const filePath = path.join(tempFolder.path, 'no_such_file.txt') const fsRead = new FsRead(features, { path: filePath }) await assert.rejects( @@ -86,7 +86,7 @@ describe('FsRead Tool', () => { it('throws error if content exceeds 30KB', async function () { const bigContent = 'x'.repeat(35_000) - const filePath = await testFolder.write('bigFile.txt', bigContent) + const filePath = await tempFolder.write('bigFile.txt', bigContent) const fsRead = new FsRead(features, { path: filePath }) await fsRead.validate() @@ -99,7 +99,7 @@ describe('FsRead Tool', () => { }) it('invalid line range', async () => { - const filePath = await testFolder.write('rangeTest.txt', '1\n2\n3') + const filePath = await tempFolder.write('rangeTest.txt', '1\n2\n3') const fsRead = new FsRead(features, { path: filePath, readRange: [3, 2] }) await fsRead.validate() diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts index 4f2874a180..d49da16475 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ import { AppendParams, CreateParams, FsWrite, InsertParams, StrReplaceParams } from './fsWrite' -import { TestFolder } from '@aws/lsp-core' +import { testFolder } from '@aws/lsp-core' import * as path from 'path' import * as assert from 'assert' import * as fs from 'fs/promises' @@ -13,7 +13,7 @@ import { Workspace } from '@aws/language-server-runtimes/server-interface' import { StubbedInstance } from 'ts-sinon' describe('FsWrite Tool', function () { - let testFolder: TestFolder + let tempFolder: testFolder.TestFolder let features: TestFeatures const expectedOutput: InvokeOutput = { output: { @@ -43,16 +43,16 @@ describe('FsWrite Tool', function () { .catch(() => false), } as Workspace['fs'], } as StubbedInstance - testFolder = await TestFolder.create() + tempFolder = await testFolder.TestFolder.create() }) after(async function () { - await testFolder.delete() + await tempFolder.delete() }) describe('handleCreate', function () { it('creates a new file with fileText content', async function () { - const filePath = path.join(testFolder.path, 'file1.txt') + const filePath = path.join(tempFolder.path, 'file1.txt') const fileExists = await features.workspace.fs.exists(filePath) assert.ok(!fileExists) @@ -71,7 +71,7 @@ describe('FsWrite Tool', function () { }) it('replaces existing file with fileText content', async function () { - const filePath = path.join(testFolder.path, 'file1.txt') + const filePath = path.join(tempFolder.path, 'file1.txt') const fileExists = await features.workspace.fs.exists(filePath) assert.ok(fileExists) @@ -90,7 +90,7 @@ describe('FsWrite Tool', function () { }) it('uses newStr when fileText is not provided', async function () { - const filePath = path.join(testFolder.path, 'file2.txt') + const filePath = path.join(tempFolder.path, 'file2.txt') const params: CreateParams = { command: 'create', @@ -107,7 +107,7 @@ describe('FsWrite Tool', function () { }) it('creates an empty file when no content is provided', async function () { - const filePath = path.join(testFolder.path, 'file3.txt') + const filePath = path.join(tempFolder.path, 'file3.txt') const params: CreateParams = { command: 'create', @@ -125,11 +125,11 @@ describe('FsWrite Tool', function () { describe('handleStrReplace', async function () { before(async function () { - testFolder = await TestFolder.create() + tempFolder = await testFolder.TestFolder.create() }) it('replaces a single occurrence of a string', async function () { - const filePath = path.join(testFolder.path, 'file1.txt') + const filePath = path.join(tempFolder.path, 'file1.txt') await fs.writeFile(filePath, 'Hello World') const params: StrReplaceParams = { @@ -148,7 +148,7 @@ describe('FsWrite Tool', function () { }) it('throws error when no matches are found', async function () { - const filePath = path.join(testFolder.path, 'file1.txt') + const filePath = path.join(tempFolder.path, 'file1.txt') const params: StrReplaceParams = { command: 'strReplace', @@ -162,7 +162,7 @@ describe('FsWrite Tool', function () { }) it('throws error when multiple matches are found', async function () { - const filePath = path.join(testFolder.path, 'file2.txt') + const filePath = path.join(tempFolder.path, 'file2.txt') await fs.writeFile(filePath, 'Hello Hello World') const params: StrReplaceParams = { @@ -180,7 +180,7 @@ describe('FsWrite Tool', function () { }) it('handles regular expression special characters correctly', async function () { - const filePath = path.join(testFolder.path, 'file3.txt') + const filePath = path.join(tempFolder.path, 'file3.txt') await fs.writeFile(filePath, 'Text with special chars: .*+?^${}()|[]\\') const params: StrReplaceParams = { @@ -199,7 +199,7 @@ describe('FsWrite Tool', function () { }) it('preserves whitespace and newlines during replacement', async function () { - const filePath = path.join(testFolder.path, 'file4.txt') + const filePath = path.join(tempFolder.path, 'file4.txt') await fs.writeFile(filePath, 'Line 1\n Indented line\nLine 3') const params: StrReplaceParams = { @@ -220,11 +220,11 @@ describe('FsWrite Tool', function () { describe('handleInsert', function () { before(async function () { - testFolder = await TestFolder.create() + tempFolder = await testFolder.TestFolder.create() }) it('inserts text after the specified line number', async function () { - const filePath = path.join(testFolder.path, 'file1.txt') + const filePath = path.join(tempFolder.path, 'file1.txt') await fs.writeFile(filePath, 'Line 1\nLine 2\nLine 3\nLine 4') const params: InsertParams = { @@ -243,7 +243,7 @@ describe('FsWrite Tool', function () { }) it('inserts text at the beginning when line number is 0', async function () { - const filePath = path.join(testFolder.path, 'file1.txt') + const filePath = path.join(tempFolder.path, 'file1.txt') const params: InsertParams = { command: 'insert', path: filePath, @@ -260,7 +260,7 @@ describe('FsWrite Tool', function () { }) it('inserts text at the end when line number exceeds file length', async function () { - const filePath = path.join(testFolder.path, 'file1.txt') + const filePath = path.join(tempFolder.path, 'file1.txt') const params: InsertParams = { command: 'insert', path: filePath, @@ -277,7 +277,7 @@ describe('FsWrite Tool', function () { }) it('handles insertion into an empty file', async function () { - const filePath = path.join(testFolder.path, 'file2.txt') + const filePath = path.join(tempFolder.path, 'file2.txt') await fs.writeFile(filePath, '') const params: InsertParams = { @@ -296,7 +296,7 @@ describe('FsWrite Tool', function () { }) it('handles negative line numbers by inserting at the beginning', async function () { - const filePath = path.join(testFolder.path, 'file2.txt') + const filePath = path.join(tempFolder.path, 'file2.txt') const params: InsertParams = { command: 'insert', @@ -314,7 +314,7 @@ describe('FsWrite Tool', function () { }) it('throws error when file does not exist', async function () { - const filePath = path.join(testFolder.path, 'nonexistent.txt') + const filePath = path.join(tempFolder.path, 'nonexistent.txt') const params: InsertParams = { command: 'insert', @@ -330,7 +330,7 @@ describe('FsWrite Tool', function () { describe('handleAppend', function () { it('appends text to the end of a file', async function () { - const filePath = path.join(testFolder.path, 'file1.txt') + const filePath = path.join(tempFolder.path, 'file1.txt') await fs.writeFile(filePath, 'Line 1\nLine 2\nLine 3\n') const params: AppendParams = { @@ -349,7 +349,7 @@ describe('FsWrite Tool', function () { }) it('adds a newline before appending if file does not end with one', async function () { - const filePath = path.join(testFolder.path, 'file2.txt') + const filePath = path.join(tempFolder.path, 'file2.txt') await fs.writeFile(filePath, 'Line 1\nLine 2\nLine 3') const params: AppendParams = { @@ -368,7 +368,7 @@ describe('FsWrite Tool', function () { }) it('appends to an empty file', async function () { - const filePath = path.join(testFolder.path, 'file3.txt') + const filePath = path.join(tempFolder.path, 'file3.txt') await fs.writeFile(filePath, '') const params: AppendParams = { @@ -386,7 +386,7 @@ describe('FsWrite Tool', function () { }) it('appends multiple lines correctly', async function () { - const filePath = path.join(testFolder.path, 'file3.txt') + const filePath = path.join(tempFolder.path, 'file3.txt') const params: AppendParams = { command: 'append', @@ -403,7 +403,7 @@ describe('FsWrite Tool', function () { }) it('handles appending empty string', async function () { - const filePath = path.join(testFolder.path, 'file3.txt') + const filePath = path.join(tempFolder.path, 'file3.txt') const params: AppendParams = { command: 'append', @@ -420,7 +420,7 @@ describe('FsWrite Tool', function () { }) it('throws error when file does not exist', async function () { - const filePath = path.join(testFolder.path, 'nonexistent.txt') + const filePath = path.join(tempFolder.path, 'nonexistent.txt') const params: AppendParams = { command: 'append', From 216db07fe282223db71ac3dde11188951078ded8 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 1 Apr 2025 13:16:59 -0400 Subject: [PATCH 11/20] refactor: pass params at invoke time --- .../agenticChat/tools/fsRead.test.ts | 36 ++++---- .../agenticChat/tools/fsRead.ts | 54 ++++++------ .../agenticChat/tools/fsWrite.test.ts | 84 +++++++++---------- .../agenticChat/tools/fsWrite.ts | 37 ++++---- 4 files changed, 103 insertions(+), 108 deletions(-) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts index 1f98c97107..9f020d8502 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts @@ -44,17 +44,21 @@ describe('FsRead Tool', () => { }) it('throws if path is empty', async () => { - const fsRead = new FsRead(features, { path: '' }) - await assert.rejects(fsRead.validate(), /Path cannot be empty/i, 'Expected an error about empty path') + const fsRead = new FsRead(features) + await assert.rejects( + fsRead.validate({ path: '' }), + /Path cannot be empty/i, + 'Expected an error about empty path' + ) }) it('reads entire file', async () => { const fileContent = 'Line 1\nLine 2\nLine 3' const filePath = await tempFolder.write('fullFile.txt', fileContent) - const fsRead = new FsRead(features, { path: filePath }) - await fsRead.validate() - const result = await fsRead.invoke(stdout) + const fsRead = new FsRead(features) + await fsRead.validate({ path: filePath }) + const result = await fsRead.invoke(stdout, { path: filePath }) assert.strictEqual(result.output.kind, 'text', 'Output kind should be "text"') assert.strictEqual(result.output.content, fileContent, 'File content should match exactly') @@ -64,9 +68,9 @@ describe('FsRead Tool', () => { const fileContent = 'A\nB\nC\nD\nE\nF' const filePath = await tempFolder.write('partialFile.txt', fileContent) - const fsRead = new FsRead(features, { path: filePath, readRange: [2, 4] }) - await fsRead.validate() - const result = await fsRead.invoke(stdout) + const fsRead = new FsRead(features) + await fsRead.validate({ path: filePath, readRange: [2, 4] }) + const result = await fsRead.invoke(stdout, { path: filePath, readRange: [2, 4] }) assert.strictEqual(result.output.kind, 'text') assert.strictEqual(result.output.content, 'B\nC\nD') @@ -74,10 +78,10 @@ describe('FsRead Tool', () => { it('throws error if path does not exist', async () => { const filePath = path.join(tempFolder.path, 'no_such_file.txt') - const fsRead = new FsRead(features, { path: filePath }) + const fsRead = new FsRead(features) await assert.rejects( - fsRead.validate(), + fsRead.validate({ path: filePath }), /does not exist or cannot be accessed/i, 'Expected an error indicating the path does not exist' ) @@ -88,11 +92,11 @@ describe('FsRead Tool', () => { const filePath = await tempFolder.write('bigFile.txt', bigContent) - const fsRead = new FsRead(features, { path: filePath }) - await fsRead.validate() + const fsRead = new FsRead(features) + await fsRead.validate({ path: filePath }) await assert.rejects( - fsRead.invoke(stdout), + fsRead.invoke(stdout, { path: filePath }), /This tool only supports reading \d+ bytes at a time/i, 'Expected a size-limit error' ) @@ -100,10 +104,10 @@ describe('FsRead Tool', () => { it('invalid line range', async () => { const filePath = await tempFolder.write('rangeTest.txt', '1\n2\n3') - const fsRead = new FsRead(features, { path: filePath, readRange: [3, 2] }) + const fsRead = new FsRead(features) - await fsRead.validate() - const result = await fsRead.invoke(stdout) + await fsRead.validate({ path: filePath, readRange: [3, 2] }) + const result = await fsRead.invoke(stdout, { path: filePath, readRange: [3, 2] }) assert.strictEqual(result.output.kind, 'text') assert.strictEqual(result.output.content, '') }) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts index 2cd81d4a91..700fba4bfb 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts @@ -14,45 +14,41 @@ export interface FsReadParams { } export class FsRead { - private fsPath: string - private readonly readRange?: number[] private readonly logging: Features['logging'] private readonly workspace: Features['workspace'] - constructor(features: Pick & Partial, params: FsReadParams) { - this.fsPath = params.path - this.readRange = params.readRange + constructor(features: Pick & Partial) { this.logging = features.logging this.workspace = features.workspace } - public async validate(): Promise { - this.logging.log(`Validating fsPath: ${this.fsPath}`) - if (!this.fsPath || this.fsPath.trim().length === 0) { + public async validate(params: FsReadParams): Promise { + this.logging.log(`Validating fsPath: ${params.path}`) + if (!params.path || params.path.trim().length === 0) { throw new Error('Path cannot be empty.') } - const sanitized = sanitize(this.fsPath) - this.fsPath = sanitized + const sanitized = sanitize(params.path) + params.path = sanitized let fileExists: boolean try { - fileExists = await this.workspace.fs.exists(this.fsPath) + fileExists = await this.workspace.fs.exists(params.path) if (!fileExists) { - throw new Error(`Path: "${this.fsPath}" does not exist or cannot be accessed.`) + throw new Error(`Path: "${params.path}" does not exist or cannot be accessed.`) } } catch (err) { - throw new Error(`Path: "${this.fsPath}" does not exist or cannot be accessed. (${err})`) + throw new Error(`Path: "${params.path}" does not exist or cannot be accessed. (${err})`) } - this.logging.log(`Validation succeeded for path: ${this.fsPath}`) + this.logging.log(`Validation succeeded for path: ${params.path}`) } - public async queueDescription(updates: WritableStream): Promise { + public async queueDescription(updates: WritableStream, params: FsReadParams): Promise { const writer = updates.getWriter() - await writer.write(`Reading file: (${this.fsPath}), `) + await writer.write(`Reading file: (${params.path}), `) - const [start, end] = this.readRange ?? [] + const [start, end] = params.readRange ?? [] if (start && end) { await writer.write(`from line ${start} to ${end}`) @@ -68,36 +64,36 @@ export class FsRead { await writer.close() } - public async invoke(_updates: WritableStream): Promise { + public async invoke(_updates: WritableStream, params: FsReadParams): Promise { try { - const fileContents = await this.readFile(this.fsPath) - this.logging.info(`Read file: ${this.fsPath}, size: ${fileContents.length}`) - return this.handleFileRange(fileContents) + const fileContents = await this.readFile(params.path) + this.logging.info(`Read file: ${params.path}, size: ${fileContents.length}`) + return this.handleFileRange(params, fileContents) } catch (error: any) { - this.logging.error(`Failed to read "${this.fsPath}": ${error.message || error}`) - throw new Error(`Failed to read "${this.fsPath}": ${error.message || error}`) + this.logging.error(`Failed to read "${params.path}": ${error.message || error}`) + throw new Error(`Failed to read "${params.path}": ${error.message || error}`) } } private async readFile(filePath: string): Promise { this.logging.info(`Reading file: ${filePath}`) - return await this.workspace.fs.readFile(this.fsPath) + return await this.workspace.fs.readFile(filePath) } - private handleFileRange(fullText: string): InvokeOutput { - if (!this.readRange || this.readRange.length === 0) { + private handleFileRange(params: FsReadParams, fullText: string): InvokeOutput { + if (!params.readRange || params.readRange.length === 0) { this.logging.log('No range provided. returning entire file.') return this.createOutput(this.enforceMaxSize(fullText)) } const lines = fullText.split('\n') - const [start, end] = this.parseLineRange(lines.length, this.readRange) + const [start, end] = this.parseLineRange(lines.length, params.readRange) if (start > end) { - this.logging.error(`Invalid range: ${this.readRange.join('-')}`) + this.logging.error(`Invalid range: ${params.readRange.join('-')}`) return this.createOutput('') } - this.logging.log(`Reading file: ${this.fsPath}, lines ${start + 1}-${end + 1}`) + this.logging.log(`Reading file: ${params.path}, lines ${start + 1}-${end + 1}`) const slice = lines.slice(start, end + 1).join('\n') return this.createOutput(this.enforceMaxSize(slice)) } diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts index d49da16475..e57d884ccb 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts @@ -61,8 +61,8 @@ describe('FsWrite Tool', function () { fileText: 'Hello World', path: filePath, } - const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(stdout) + const fsWrite = new FsWrite(features) + const output = await fsWrite.invoke(stdout, params) const content = await features.workspace.fs.readFile(filePath) assert.strictEqual(content, 'Hello World') @@ -80,8 +80,8 @@ describe('FsWrite Tool', function () { fileText: 'Goodbye', path: filePath, } - const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(stdout) + const fsWrite = new FsWrite(features) + const output = await fsWrite.invoke(stdout, params) const content = await features.workspace.fs.readFile(filePath) assert.strictEqual(content, 'Goodbye') @@ -97,8 +97,8 @@ describe('FsWrite Tool', function () { newStr: 'Hello World', path: filePath, } - const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(stdout) + const fsWrite = new FsWrite(features) + const output = await fsWrite.invoke(stdout, params) const content = await features.workspace.fs.readFile(filePath) assert.strictEqual(content, 'Hello World') @@ -113,8 +113,8 @@ describe('FsWrite Tool', function () { command: 'create', path: filePath, } - const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(stdout) + const fsWrite = new FsWrite(features) + const output = await fsWrite.invoke(stdout, params) const content = await features.workspace.fs.readFile(filePath) assert.strictEqual(content, '') @@ -138,8 +138,8 @@ describe('FsWrite Tool', function () { oldStr: 'Hello', newStr: 'Goodbye', } - const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(stdout) + const fsWrite = new FsWrite(features) + const output = await fsWrite.invoke(stdout, params) const content = await features.workspace.fs.readFile(filePath) assert.strictEqual(content, 'Goodbye World') @@ -157,8 +157,8 @@ describe('FsWrite Tool', function () { newStr: 'Goodbye', } - const fsWrite = new FsWrite(features, params) - await assert.rejects(() => fsWrite.invoke(stdout), /No occurrences of "Invalid" were found/) + const fsWrite = new FsWrite(features) + await assert.rejects(() => fsWrite.invoke(stdout, params), /No occurrences of "Invalid" were found/) }) it('throws error when multiple matches are found', async function () { @@ -172,9 +172,9 @@ describe('FsWrite Tool', function () { newStr: 'Goodbye', } - const fsWrite = new FsWrite(features, params) + const fsWrite = new FsWrite(features) await assert.rejects( - () => fsWrite.invoke(stdout), + () => fsWrite.invoke(stdout, params), /2 occurrences of oldStr were found when only 1 is expected/ ) }) @@ -189,8 +189,8 @@ describe('FsWrite Tool', function () { oldStr: '.*+?^${}()|[]\\', newStr: 'REPLACED', } - const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(stdout) + const fsWrite = new FsWrite(features) + const output = await fsWrite.invoke(stdout, params) const content = await features.workspace.fs.readFile(filePath) assert.strictEqual(content, 'Text with special chars: REPLACED') @@ -208,8 +208,8 @@ describe('FsWrite Tool', function () { oldStr: ' Indented line\n', newStr: ' Double indented\n', } - const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(stdout) + const fsWrite = new FsWrite(features) + const output = await fsWrite.invoke(stdout, params) const content = await features.workspace.fs.readFile(filePath) assert.strictEqual(content, 'Line 1\n Double indented\nLine 3') @@ -233,8 +233,8 @@ describe('FsWrite Tool', function () { insertLine: 2, newStr: 'New Line', } - const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(stdout) + const fsWrite = new FsWrite(features) + const output = await fsWrite.invoke(stdout, params) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'Line 1\nLine 2\nNew Line\nLine 3\nLine 4') @@ -250,8 +250,8 @@ describe('FsWrite Tool', function () { insertLine: 0, newStr: 'New First Line', } - const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(stdout) + const fsWrite = new FsWrite(features) + const output = await fsWrite.invoke(stdout, params) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'New First Line\nLine 1\nLine 2\nNew Line\nLine 3\nLine 4') @@ -267,8 +267,8 @@ describe('FsWrite Tool', function () { insertLine: 10, newStr: 'New Last Line', } - const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(stdout) + const fsWrite = new FsWrite(features) + const output = await fsWrite.invoke(stdout, params) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'New First Line\nLine 1\nLine 2\nNew Line\nLine 3\nLine 4\nNew Last Line') @@ -286,8 +286,8 @@ describe('FsWrite Tool', function () { insertLine: 0, newStr: 'First Line', } - const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(stdout) + const fsWrite = new FsWrite(features) + const output = await fsWrite.invoke(stdout, params) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'First Line\n') @@ -304,8 +304,8 @@ describe('FsWrite Tool', function () { insertLine: -1, newStr: 'New First Line', } - const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(stdout) + const fsWrite = new FsWrite(features) + const output = await fsWrite.invoke(stdout, params) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'New First Line\nFirst Line\n') @@ -323,8 +323,8 @@ describe('FsWrite Tool', function () { newStr: 'New Line', } - const fsWrite = new FsWrite(features, params) - await assert.rejects(() => fsWrite.invoke(stdout), /no such file or directory/) + const fsWrite = new FsWrite(features) + await assert.rejects(() => fsWrite.invoke(stdout, params), /no such file or directory/) }) }) @@ -339,8 +339,8 @@ describe('FsWrite Tool', function () { newStr: 'Line 4', } - const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(stdout) + const fsWrite = new FsWrite(features) + const output = await fsWrite.invoke(stdout, params) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'Line 1\nLine 2\nLine 3\nLine 4') @@ -358,8 +358,8 @@ describe('FsWrite Tool', function () { newStr: 'Line 4', } - const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(stdout) + const fsWrite = new FsWrite(features) + const output = await fsWrite.invoke(stdout, params) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'Line 1\nLine 2\nLine 3\nLine 4') @@ -376,8 +376,8 @@ describe('FsWrite Tool', function () { path: filePath, newStr: 'Line 1', } - const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(stdout) + const fsWrite = new FsWrite(features) + const output = await fsWrite.invoke(stdout, params) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'Line 1') @@ -393,8 +393,8 @@ describe('FsWrite Tool', function () { path: filePath, newStr: 'Line 2\nLine 3', } - const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(stdout) + const fsWrite = new FsWrite(features) + const output = await fsWrite.invoke(stdout, params) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'Line 1\nLine 2\nLine 3') @@ -410,8 +410,8 @@ describe('FsWrite Tool', function () { path: filePath, newStr: '', } - const fsWrite = new FsWrite(features, params) - const output = await fsWrite.invoke(stdout) + const fsWrite = new FsWrite(features) + const output = await fsWrite.invoke(stdout, params) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'Line 1\nLine 2\nLine 3\n') @@ -428,8 +428,8 @@ describe('FsWrite Tool', function () { newStr: 'New Line', } - const fsWrite = new FsWrite(features, params) - await assert.rejects(() => fsWrite.invoke(stdout), /no such file or directory/) + const fsWrite = new FsWrite(features) + await assert.rejects(() => fsWrite.invoke(stdout, params), /no such file or directory/) }) }) }) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts index ad4f1a62bc..3303f32314 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts @@ -34,34 +34,29 @@ export interface AppendParams extends BaseParams { export type FsWriteParams = CreateParams | StrReplaceParams | InsertParams | AppendParams export class FsWrite { - private fsPath: string private readonly logging: Features['logging'] private readonly workspace: Features['workspace'] - constructor( - features: Pick & Partial, - readonly params: FsWriteParams - ) { - this.fsPath = params.path + constructor(features: Pick & Partial) { this.logging = features.logging this.workspace = features.workspace } - public async invoke(_updates: WritableStream): Promise { - const sanitizedPath = sanitize(this.params.path) + public async invoke(_updates: WritableStream, params: FsWriteParams): Promise { + const sanitizedPath = sanitize(params.path) - switch (this.params.command) { + switch (params.command) { case 'create': - await this.handleCreate(this.params, sanitizedPath) + await this.handleCreate(params, sanitizedPath) break case 'strReplace': - await this.handleStrReplace(this.params, sanitizedPath) + await this.handleStrReplace(params, sanitizedPath) break case 'insert': - await this.handleInsert(this.params, sanitizedPath) + await this.handleInsert(params, sanitizedPath) break case 'append': - await this.handleAppend(this.params, sanitizedPath) + await this.handleAppend(params, sanitizedPath) break } @@ -73,32 +68,32 @@ export class FsWrite { } } - public async queueDescription(updates: WritableStream): Promise { + public async queueDescription(updates: WritableStream, params: FsWriteParams): Promise { const writer = updates.getWriter() - await writer.write(`Writing to: (${this.params.path})`) + await writer.write(`Writing to: (${params.path})`) await writer.close() } - public async validate(): Promise { - switch (this.params.command) { + public async validate(params: FsWriteParams): Promise { + switch (params.command) { case 'create': - if (!this.params.path) { + if (!params.path) { throw new Error('Path must not be empty') } break case 'strReplace': case 'insert': { - const fileExists = await this.workspace.fs.exists(this.params.path) + const fileExists = await this.workspace.fs.exists(params.path) if (!fileExists) { throw new Error('The provided path must exist in order to replace or insert contents into it') } break } case 'append': - if (!this.params.path) { + if (!params.path) { throw new Error('Path must not be empty') } - if (!this.params.newStr) { + if (!params.newStr) { throw new Error('Content to append must not be empty') } break From 3b7bf1517101e9f9ad72064e331dbbca9ce59ccd Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 1 Apr 2025 15:15:24 -0400 Subject: [PATCH 12/20] feat: add fs tools server --- .../agenticChat/tools/fsRead.test.ts | 8 ++-- .../agenticChat/tools/fsRead.ts | 28 ++++++++++- .../agenticChat/tools/fsWrite.test.ts | 42 ++++++++--------- .../agenticChat/tools/fsWrite.ts | 46 ++++++++++++++++++- .../agenticChat/tools/toolServer.ts | 19 ++++++++ 5 files changed, 116 insertions(+), 27 deletions(-) create mode 100644 server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolServer.ts diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts index 9f020d8502..e75c02d8d2 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts @@ -58,7 +58,7 @@ describe('FsRead Tool', () => { const fsRead = new FsRead(features) await fsRead.validate({ path: filePath }) - const result = await fsRead.invoke(stdout, { path: filePath }) + const result = await fsRead.invoke({ path: filePath }) assert.strictEqual(result.output.kind, 'text', 'Output kind should be "text"') assert.strictEqual(result.output.content, fileContent, 'File content should match exactly') @@ -70,7 +70,7 @@ describe('FsRead Tool', () => { const fsRead = new FsRead(features) await fsRead.validate({ path: filePath, readRange: [2, 4] }) - const result = await fsRead.invoke(stdout, { path: filePath, readRange: [2, 4] }) + const result = await fsRead.invoke({ path: filePath, readRange: [2, 4] }) assert.strictEqual(result.output.kind, 'text') assert.strictEqual(result.output.content, 'B\nC\nD') @@ -96,7 +96,7 @@ describe('FsRead Tool', () => { await fsRead.validate({ path: filePath }) await assert.rejects( - fsRead.invoke(stdout, { path: filePath }), + fsRead.invoke({ path: filePath }), /This tool only supports reading \d+ bytes at a time/i, 'Expected a size-limit error' ) @@ -107,7 +107,7 @@ describe('FsRead Tool', () => { const fsRead = new FsRead(features) await fsRead.validate({ path: filePath, readRange: [3, 2] }) - const result = await fsRead.invoke(stdout, { path: filePath, readRange: [3, 2] }) + const result = await fsRead.invoke({ path: filePath, readRange: [3, 2] }) assert.strictEqual(result.output.kind, 'text') assert.strictEqual(result.output.content, '') }) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts index 700fba4bfb..5cf4c86798 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts @@ -64,7 +64,7 @@ export class FsRead { await writer.close() } - public async invoke(_updates: WritableStream, params: FsReadParams): Promise { + public async invoke(params: FsReadParams): Promise { try { const fileContents = await this.readFile(params.path) this.logging.info(`Read file: ${params.path}, size: ${fileContents.length}`) @@ -134,4 +134,30 @@ export class FsRead { }, } } + + public static getSpec() { + return { + name: 'fsRead', + description: + 'A tool for reading a file. \n* This tool returns the contents of a file, and the optional `readRange` determines what range of lines will be read from the specified file.', + inputSchema: { + type: 'object' as 'object', + parameters: { + path: { + description: 'Absolute path to a file, e.g. `/repo/file.py`.', + type: 'string', + }, + readRange: { + description: + 'Optional parameter when reading files.\n* If none is given, the full file is shown. If provided, the file will be shown in the indicated line number range, e.g. [11, 12] will show lines 11 and 12. Indexing at 1 to start. Setting `[startLine, -1]` shows all lines from `startLine` to the end of the file.', + items: { + type: 'integer', + }, + type: 'array', + }, + }, + required: ['path'], + }, + } + } } diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts index e57d884ccb..7c161f428b 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts @@ -62,7 +62,7 @@ describe('FsWrite Tool', function () { path: filePath, } const fsWrite = new FsWrite(features) - const output = await fsWrite.invoke(stdout, params) + const output = await fsWrite.invoke(params) const content = await features.workspace.fs.readFile(filePath) assert.strictEqual(content, 'Hello World') @@ -81,7 +81,7 @@ describe('FsWrite Tool', function () { path: filePath, } const fsWrite = new FsWrite(features) - const output = await fsWrite.invoke(stdout, params) + const output = await fsWrite.invoke(params) const content = await features.workspace.fs.readFile(filePath) assert.strictEqual(content, 'Goodbye') @@ -98,7 +98,7 @@ describe('FsWrite Tool', function () { path: filePath, } const fsWrite = new FsWrite(features) - const output = await fsWrite.invoke(stdout, params) + const output = await fsWrite.invoke(params) const content = await features.workspace.fs.readFile(filePath) assert.strictEqual(content, 'Hello World') @@ -114,7 +114,7 @@ describe('FsWrite Tool', function () { path: filePath, } const fsWrite = new FsWrite(features) - const output = await fsWrite.invoke(stdout, params) + const output = await fsWrite.invoke(params) const content = await features.workspace.fs.readFile(filePath) assert.strictEqual(content, '') @@ -139,7 +139,7 @@ describe('FsWrite Tool', function () { newStr: 'Goodbye', } const fsWrite = new FsWrite(features) - const output = await fsWrite.invoke(stdout, params) + const output = await fsWrite.invoke(params) const content = await features.workspace.fs.readFile(filePath) assert.strictEqual(content, 'Goodbye World') @@ -158,7 +158,7 @@ describe('FsWrite Tool', function () { } const fsWrite = new FsWrite(features) - await assert.rejects(() => fsWrite.invoke(stdout, params), /No occurrences of "Invalid" were found/) + await assert.rejects(() => fsWrite.invoke(params), /No occurrences of "Invalid" were found/) }) it('throws error when multiple matches are found', async function () { @@ -174,7 +174,7 @@ describe('FsWrite Tool', function () { const fsWrite = new FsWrite(features) await assert.rejects( - () => fsWrite.invoke(stdout, params), + () => fsWrite.invoke(params), /2 occurrences of oldStr were found when only 1 is expected/ ) }) @@ -190,7 +190,7 @@ describe('FsWrite Tool', function () { newStr: 'REPLACED', } const fsWrite = new FsWrite(features) - const output = await fsWrite.invoke(stdout, params) + const output = await fsWrite.invoke(params) const content = await features.workspace.fs.readFile(filePath) assert.strictEqual(content, 'Text with special chars: REPLACED') @@ -209,7 +209,7 @@ describe('FsWrite Tool', function () { newStr: ' Double indented\n', } const fsWrite = new FsWrite(features) - const output = await fsWrite.invoke(stdout, params) + const output = await fsWrite.invoke(params) const content = await features.workspace.fs.readFile(filePath) assert.strictEqual(content, 'Line 1\n Double indented\nLine 3') @@ -234,7 +234,7 @@ describe('FsWrite Tool', function () { newStr: 'New Line', } const fsWrite = new FsWrite(features) - const output = await fsWrite.invoke(stdout, params) + const output = await fsWrite.invoke(params) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'Line 1\nLine 2\nNew Line\nLine 3\nLine 4') @@ -251,7 +251,7 @@ describe('FsWrite Tool', function () { newStr: 'New First Line', } const fsWrite = new FsWrite(features) - const output = await fsWrite.invoke(stdout, params) + const output = await fsWrite.invoke(params) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'New First Line\nLine 1\nLine 2\nNew Line\nLine 3\nLine 4') @@ -268,7 +268,7 @@ describe('FsWrite Tool', function () { newStr: 'New Last Line', } const fsWrite = new FsWrite(features) - const output = await fsWrite.invoke(stdout, params) + const output = await fsWrite.invoke(params) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'New First Line\nLine 1\nLine 2\nNew Line\nLine 3\nLine 4\nNew Last Line') @@ -287,7 +287,7 @@ describe('FsWrite Tool', function () { newStr: 'First Line', } const fsWrite = new FsWrite(features) - const output = await fsWrite.invoke(stdout, params) + const output = await fsWrite.invoke(params) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'First Line\n') @@ -305,7 +305,7 @@ describe('FsWrite Tool', function () { newStr: 'New First Line', } const fsWrite = new FsWrite(features) - const output = await fsWrite.invoke(stdout, params) + const output = await fsWrite.invoke(params) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'New First Line\nFirst Line\n') @@ -324,7 +324,7 @@ describe('FsWrite Tool', function () { } const fsWrite = new FsWrite(features) - await assert.rejects(() => fsWrite.invoke(stdout, params), /no such file or directory/) + await assert.rejects(() => fsWrite.invoke(params), /no such file or directory/) }) }) @@ -340,7 +340,7 @@ describe('FsWrite Tool', function () { } const fsWrite = new FsWrite(features) - const output = await fsWrite.invoke(stdout, params) + const output = await fsWrite.invoke(params) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'Line 1\nLine 2\nLine 3\nLine 4') @@ -359,7 +359,7 @@ describe('FsWrite Tool', function () { } const fsWrite = new FsWrite(features) - const output = await fsWrite.invoke(stdout, params) + const output = await fsWrite.invoke(params) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'Line 1\nLine 2\nLine 3\nLine 4') @@ -377,7 +377,7 @@ describe('FsWrite Tool', function () { newStr: 'Line 1', } const fsWrite = new FsWrite(features) - const output = await fsWrite.invoke(stdout, params) + const output = await fsWrite.invoke(params) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'Line 1') @@ -394,7 +394,7 @@ describe('FsWrite Tool', function () { newStr: 'Line 2\nLine 3', } const fsWrite = new FsWrite(features) - const output = await fsWrite.invoke(stdout, params) + const output = await fsWrite.invoke(params) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'Line 1\nLine 2\nLine 3') @@ -411,7 +411,7 @@ describe('FsWrite Tool', function () { newStr: '', } const fsWrite = new FsWrite(features) - const output = await fsWrite.invoke(stdout, params) + const output = await fsWrite.invoke(params) const newContent = await features.workspace.fs.readFile(filePath) assert.strictEqual(newContent, 'Line 1\nLine 2\nLine 3\n') @@ -429,7 +429,7 @@ describe('FsWrite Tool', function () { } const fsWrite = new FsWrite(features) - await assert.rejects(() => fsWrite.invoke(stdout, params), /no such file or directory/) + await assert.rejects(() => fsWrite.invoke(params), /no such file or directory/) }) }) }) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts index 3303f32314..5057aa461f 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts @@ -42,7 +42,7 @@ export class FsWrite { this.workspace = features.workspace } - public async invoke(_updates: WritableStream, params: FsWriteParams): Promise { + public async invoke(params: FsWriteParams): Promise { const sanitizedPath = sanitize(params.path) switch (params.command) { @@ -167,4 +167,48 @@ export class FsWrite { private escapeRegExp(string: string): string { return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') } + + public static getSpec() { + return { + name: 'fsWrite', + description: + 'A tool for creating and editing a file.\n * The `create` command will override the file at `path` if it already exists as a file, and otherwise create a new file\n * The `append` command will add content to the end of an existing file, automatically adding a newline if the file does not end with one. The file must exist.\n Notes for using the `strReplace` command:\n * The `oldStr` parameter should match EXACTLY one or more consecutive lines from the original file. Be mindful of whitespaces!\n * If the `oldStr` parameter is not unique in the file, the replacement will not be performed. Make sure to include enough context in `oldStr` to make it unique\n * The `newStr` parameter should contain the edited lines that should replace the `oldStr`. The `insert` command will insert `newStr` after `insertLine` and place it on its own line.', + inputSchema: { + type: 'object' as 'object', + parameters: { + command: { + type: 'string', + enum: ['create', 'strReplace', 'insert', 'append'], + description: + 'The commands to run. Allowed options are: `create`, `strReplace`, `insert`, `append`.', + }, + fileText: { + description: + 'Required parameter of `create` command, with the content of the file to be created.', + type: 'string', + }, + insertLine: { + description: + 'Required parameter of `insert` command. The `newStr` will be inserted AFTER the line `insertLine` of `path`.', + type: 'integer', + }, + newStr: { + description: + 'Required parameter of `strReplace` command containing the new string. Required parameter of `insert` command containing the string to insert. Required parameter of `append` command containing the content to append to the file.', + type: 'string', + }, + oldStr: { + description: + 'Required parameter of `strReplace` command containing the string in `path` to replace.', + type: 'string', + }, + path: { + description: 'Absolute path to file or directory, e.g. `/repo/file.py` or `/repo`.', + type: 'string', + }, + }, + required: ['command', 'path'], + }, + } + } } 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 new file mode 100644 index 0000000000..2a338770f4 --- /dev/null +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolServer.ts @@ -0,0 +1,19 @@ +import { Server } from '@aws/language-server-runtimes/server-interface' +import { FsRead } from './fsRead' +import { FsWrite } from './fsWrite' + +export const FsToolsServer: Server = ({ workspace, logging, agent }) => { + const fsRead = new FsRead({ workspace, logging }) + const fsWrite = new FsWrite({ workspace, logging }) + + agent.addTool(FsRead.getSpec(), input => { + return fsRead.invoke(input) + }) + + agent.addTool(FsWrite.getSpec(), input => { + return fsWrite.invoke(input) + }) + + // nothing to dispose of. + return () => {} +} From a9b8804d6112a098719da8ed295f62a02de81370 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 1 Apr 2025 15:45:04 -0400 Subject: [PATCH 13/20] refactor: remove queue description --- .../agenticChat/tools/fsRead.ts | 20 ------------------- .../agenticChat/tools/fsWrite.ts | 6 ------ .../agenticChat/tools/toolServer.ts | 8 ++++---- 3 files changed, 4 insertions(+), 30 deletions(-) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts index 5cf4c86798..6aed352b4d 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts @@ -44,26 +44,6 @@ export class FsRead { this.logging.log(`Validation succeeded for path: ${params.path}`) } - public async queueDescription(updates: WritableStream, params: FsReadParams): Promise { - const writer = updates.getWriter() - await writer.write(`Reading file: (${params.path}), `) - - const [start, end] = params.readRange ?? [] - - if (start && end) { - await writer.write(`from line ${start} to ${end}`) - } else if (start) { - if (start > 0) { - await writer.write(`from line ${start} to end of file`) - } else { - await writer.write(`${start} line from the end of file to end of file`) - } - } else { - await writer.write('all lines') - } - await writer.close() - } - public async invoke(params: FsReadParams): Promise { try { const fileContents = await this.readFile(params.path) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts index 5057aa461f..dbe07e0301 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts @@ -68,12 +68,6 @@ export class FsWrite { } } - public async queueDescription(updates: WritableStream, params: FsWriteParams): Promise { - const writer = updates.getWriter() - await writer.write(`Writing to: (${params.path})`) - await writer.close() - } - public async validate(params: FsWriteParams): Promise { switch (params.command) { case 'create': 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 2a338770f4..8eae96b728 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 @@ -1,16 +1,16 @@ import { Server } from '@aws/language-server-runtimes/server-interface' -import { FsRead } from './fsRead' -import { FsWrite } from './fsWrite' +import { FsRead, FsReadParams } from './fsRead' +import { FsWrite, FsWriteParams } from './fsWrite' export const FsToolsServer: Server = ({ workspace, logging, agent }) => { const fsRead = new FsRead({ workspace, logging }) const fsWrite = new FsWrite({ workspace, logging }) - agent.addTool(FsRead.getSpec(), input => { + agent.addTool(FsRead.getSpec(), (input: FsReadParams) => { return fsRead.invoke(input) }) - agent.addTool(FsWrite.getSpec(), input => { + agent.addTool(FsWrite.getSpec(), (input: FsWriteParams) => { return fsWrite.invoke(input) }) From a430ad3854c0a07f1fa6c288e4a4ec9c1a36cb2d Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 1 Apr 2025 16:41:30 -0400 Subject: [PATCH 14/20] test: port path tests related to sanitize --- core/aws-lsp-core/src/util/path.test.ts | 39 ++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/core/aws-lsp-core/src/util/path.test.ts b/core/aws-lsp-core/src/util/path.test.ts index 741b8d1381..2b6114f32f 100644 --- a/core/aws-lsp-core/src/util/path.test.ts +++ b/core/aws-lsp-core/src/util/path.test.ts @@ -4,7 +4,8 @@ import * as assert from 'assert' import * as path from 'path' import * as os from 'os' -import { normalizeSeparator, normalize, isInDirectory } from './path' +import { normalizeSeparator, normalize, isInDirectory, sanitize } from './path' +import sinon from 'ts-sinon' describe('pathUtils', async function () { it('normalizeSeparator()', function () { @@ -76,4 +77,40 @@ describe('pathUtils', async function () { assert.ok(!isInDirectory('/foo/bar/baz/', '/FOO/BAR/BAZ/A.TXT')) } }) + + describe('sanitizePath', function () { + let sandbox: sinon.SinonSandbox + + beforeEach(function () { + sandbox = sinon.createSandbox() + }) + + afterEach(function () { + sandbox.restore() + }) + + it('trims whitespace from input path', function () { + const result = sanitize(' /test/path ') + assert.strictEqual(result, '/test/path') + }) + + it('expands tilde to user home directory', function () { + const homeDir = '/Users/testuser' + sandbox.stub(os, 'homedir').returns(homeDir) + + const result = sanitize('~/documents/file.txt') + assert.strictEqual(result, path.join(homeDir, 'documents/file.txt')) + }) + + it('converts relative paths to absolute paths', function () { + const result = sanitize('relative/path') + assert.strictEqual(result, path.resolve('relative/path')) + }) + + it('leaves absolute paths unchanged', function () { + const absolutePath = path.resolve('/absolute/path') + const result = sanitize(absolutePath) + assert.strictEqual(result, absolutePath) + }) + }) }) From 1949a565ae8a55933c02fb449d7f037cb33d646c Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 1 Apr 2025 16:45:12 -0400 Subject: [PATCH 15/20] refactor: avoid unnecessary export --- .../src/language-server/agenticChat/tools/fsRead.ts | 4 ++-- .../src/language-server/agenticChat/tools/fsWrite.test.ts | 4 ++-- .../src/language-server/agenticChat/tools/fsWrite.ts | 4 ++-- .../src/language-server/agenticChat/tools/toolShared.ts | 7 +------ 4 files changed, 7 insertions(+), 12 deletions(-) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts index 6aed352b4d..7ebb691f25 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts @@ -2,7 +2,7 @@ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. * SPDX-License-Identifier: Apache-2.0 */ -import { InvokeOutput, maxToolResponseSize, OutputKind } from './toolShared' +import { InvokeOutput, maxToolResponseSize } from './toolShared' import { Features } from '@aws/language-server-runtimes/server-interface/server' import { sanitize } from '@aws/lsp-core/out/util/path' @@ -109,7 +109,7 @@ export class FsRead { private createOutput(content: string): InvokeOutput { return { output: { - kind: OutputKind.Text, + kind: 'text', content: content, }, } diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts index 7c161f428b..eb45e38d10 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts @@ -7,7 +7,7 @@ import { testFolder } from '@aws/lsp-core' import * as path from 'path' import * as assert from 'assert' import * as fs from 'fs/promises' -import { InvokeOutput, OutputKind } from './toolShared' +import { InvokeOutput } from './toolShared' import { TestFeatures } from '@aws/language-server-runtimes/testing' import { Workspace } from '@aws/language-server-runtimes/server-interface' import { StubbedInstance } from 'ts-sinon' @@ -17,7 +17,7 @@ describe('FsWrite Tool', function () { let features: TestFeatures const expectedOutput: InvokeOutput = { output: { - kind: OutputKind.Text, + kind: 'text', content: '', }, } diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts index dbe07e0301..4b3f9fc3ba 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts @@ -1,4 +1,4 @@ -import { InvokeOutput, OutputKind } from './toolShared' +import { InvokeOutput } from './toolShared' import { Features } from '@aws/language-server-runtimes/server-interface/server' import { sanitize } from '@aws/lsp-core/out/util/path' @@ -62,7 +62,7 @@ export class FsWrite { return { output: { - kind: OutputKind.Text, + kind: 'text', content: '', }, } diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.ts index 24b564ea89..aa5733a912 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.ts @@ -1,13 +1,8 @@ export const maxToolResponseSize = 30720 // 30KB -export enum OutputKind { - Text = 'text', - Json = 'json', -} - export interface InvokeOutput { output: { - kind: OutputKind + kind: 'text' | 'json' content: string } } From 4c02f9ffd5c4a63c4125d9619ae13cdb7f50ae1b Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 1 Apr 2025 17:02:54 -0400 Subject: [PATCH 16/20] refactor: validate before invoking --- .../src/language-server/agenticChat/tools/fsRead.ts | 3 ++- .../language-server/agenticChat/tools/fsWrite.ts | 3 ++- .../language-server/agenticChat/tools/toolServer.ts | 13 ++++--------- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts index 7ebb691f25..9b5e64544c 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts @@ -45,6 +45,7 @@ export class FsRead { } public async invoke(params: FsReadParams): Promise { + await this.validate(params) try { const fileContents = await this.readFile(params.path) this.logging.info(`Read file: ${params.path}, size: ${fileContents.length}`) @@ -115,7 +116,7 @@ export class FsRead { } } - public static getSpec() { + public getSpec() { return { name: 'fsRead', description: diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts index 4b3f9fc3ba..b829796368 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts @@ -43,6 +43,7 @@ export class FsWrite { } public async invoke(params: FsWriteParams): Promise { + await this.validate(params) const sanitizedPath = sanitize(params.path) switch (params.command) { @@ -162,7 +163,7 @@ export class FsWrite { return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') } - public static getSpec() { + public getSpec() { return { name: 'fsWrite', description: 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 8eae96b728..610e4b8123 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 @@ -3,17 +3,12 @@ import { FsRead, FsReadParams } from './fsRead' import { FsWrite, FsWriteParams } from './fsWrite' export const FsToolsServer: Server = ({ workspace, logging, agent }) => { - const fsRead = new FsRead({ workspace, logging }) - const fsWrite = new FsWrite({ workspace, logging }) + const fsReadTool = new FsRead({ workspace, logging }) + const fsWriteTool = new FsWrite({ workspace, logging }) - agent.addTool(FsRead.getSpec(), (input: FsReadParams) => { - return fsRead.invoke(input) - }) + agent.addTool(fsReadTool.getSpec(), (input: FsReadParams) => fsReadTool.invoke(input)) - agent.addTool(FsWrite.getSpec(), (input: FsWriteParams) => { - return fsWrite.invoke(input) - }) + agent.addTool(fsWriteTool.getSpec(), (input: FsWriteParams) => fsWriteTool.invoke(input)) - // nothing to dispose of. return () => {} } From 235affcedd1b844c2bb1a2bab6da229c9bd0b5e2 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 1 Apr 2025 18:04:36 -0400 Subject: [PATCH 17/20] refactor: avoid excessive rethrowing --- .../agenticChat/tools/fsRead.test.ts | 17 ++------- .../agenticChat/tools/fsRead.ts | 37 +++---------------- .../agenticChat/tools/fsWrite.test.ts | 17 --------- .../agenticChat/tools/fsWrite.ts | 27 -------------- .../agenticChat/tools/toolServer.ts | 4 +- 5 files changed, 10 insertions(+), 92 deletions(-) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts index e75c02d8d2..1842f1dde8 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts @@ -45,11 +45,7 @@ describe('FsRead Tool', () => { it('throws if path is empty', async () => { const fsRead = new FsRead(features) - await assert.rejects( - fsRead.validate({ path: '' }), - /Path cannot be empty/i, - 'Expected an error about empty path' - ) + await assert.rejects(() => fsRead.invoke({ path: '' })) }) it('reads entire file', async () => { @@ -57,7 +53,6 @@ describe('FsRead Tool', () => { const filePath = await tempFolder.write('fullFile.txt', fileContent) const fsRead = new FsRead(features) - await fsRead.validate({ path: filePath }) const result = await fsRead.invoke({ path: filePath }) assert.strictEqual(result.output.kind, 'text', 'Output kind should be "text"') @@ -69,7 +64,6 @@ describe('FsRead Tool', () => { const filePath = await tempFolder.write('partialFile.txt', fileContent) const fsRead = new FsRead(features) - await fsRead.validate({ path: filePath, readRange: [2, 4] }) const result = await fsRead.invoke({ path: filePath, readRange: [2, 4] }) assert.strictEqual(result.output.kind, 'text') @@ -80,11 +74,7 @@ describe('FsRead Tool', () => { const filePath = path.join(tempFolder.path, 'no_such_file.txt') const fsRead = new FsRead(features) - await assert.rejects( - fsRead.validate({ path: filePath }), - /does not exist or cannot be accessed/i, - 'Expected an error indicating the path does not exist' - ) + await assert.rejects(fsRead.invoke({ path: filePath })) }) it('throws error if content exceeds 30KB', async function () { @@ -93,7 +83,6 @@ describe('FsRead Tool', () => { const filePath = await tempFolder.write('bigFile.txt', bigContent) const fsRead = new FsRead(features) - await fsRead.validate({ path: filePath }) await assert.rejects( fsRead.invoke({ path: filePath }), @@ -106,7 +95,7 @@ describe('FsRead Tool', () => { const filePath = await tempFolder.write('rangeTest.txt', '1\n2\n3') const fsRead = new FsRead(features) - await fsRead.validate({ path: filePath, readRange: [3, 2] }) + await fsRead.invoke({ path: filePath, readRange: [3, 2] }) const result = await fsRead.invoke({ path: filePath, readRange: [3, 2] }) assert.strictEqual(result.output.kind, 'text') assert.strictEqual(result.output.content, '') diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts index 9b5e64544c..61addc8785 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts @@ -2,9 +2,9 @@ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. * SPDX-License-Identifier: Apache-2.0 */ +import { sanitize } from '@aws/lsp-core/out/util/path' import { InvokeOutput, maxToolResponseSize } from './toolShared' import { Features } from '@aws/language-server-runtimes/server-interface/server' -import { sanitize } from '@aws/lsp-core/out/util/path' // Port of https://github.com/aws/aws-toolkit-vscode/blob/10bb1c7dc45f128df14d749d95905c0e9b808096/packages/core/src/codewhispererChat/tools/fsRead.ts#L17 @@ -22,38 +22,11 @@ export class FsRead { this.workspace = features.workspace } - public async validate(params: FsReadParams): Promise { - this.logging.log(`Validating fsPath: ${params.path}`) - if (!params.path || params.path.trim().length === 0) { - throw new Error('Path cannot be empty.') - } - - const sanitized = sanitize(params.path) - params.path = sanitized - - let fileExists: boolean - try { - fileExists = await this.workspace.fs.exists(params.path) - if (!fileExists) { - throw new Error(`Path: "${params.path}" does not exist or cannot be accessed.`) - } - } catch (err) { - throw new Error(`Path: "${params.path}" does not exist or cannot be accessed. (${err})`) - } - - this.logging.log(`Validation succeeded for path: ${params.path}`) - } - public async invoke(params: FsReadParams): Promise { - await this.validate(params) - try { - const fileContents = await this.readFile(params.path) - this.logging.info(`Read file: ${params.path}, size: ${fileContents.length}`) - return this.handleFileRange(params, fileContents) - } catch (error: any) { - this.logging.error(`Failed to read "${params.path}": ${error.message || error}`) - throw new Error(`Failed to read "${params.path}": ${error.message || error}`) - } + const path = sanitize(params.path) + const fileContents = await this.readFile(path) + this.logging.info(`Read file: ${path}, size: ${fileContents.length}`) + return this.handleFileRange(params, fileContents) } private async readFile(filePath: string): Promise { diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts index eb45e38d10..4a75ad3e3d 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.test.ts @@ -402,23 +402,6 @@ describe('FsWrite Tool', function () { assert.deepStrictEqual(output, expectedOutput) }) - it('handles appending empty string', async function () { - const filePath = path.join(tempFolder.path, 'file3.txt') - - const params: AppendParams = { - command: 'append', - path: filePath, - newStr: '', - } - const fsWrite = new FsWrite(features) - const output = await fsWrite.invoke(params) - - const newContent = await features.workspace.fs.readFile(filePath) - assert.strictEqual(newContent, 'Line 1\nLine 2\nLine 3\n') - - assert.deepStrictEqual(output, expectedOutput) - }) - it('throws error when file does not exist', async function () { const filePath = path.join(tempFolder.path, 'nonexistent.txt') diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts index b829796368..1845375e46 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts @@ -43,7 +43,6 @@ export class FsWrite { } public async invoke(params: FsWriteParams): Promise { - await this.validate(params) const sanitizedPath = sanitize(params.path) switch (params.command) { @@ -69,32 +68,6 @@ export class FsWrite { } } - public async validate(params: FsWriteParams): Promise { - switch (params.command) { - case 'create': - if (!params.path) { - throw new Error('Path must not be empty') - } - break - case 'strReplace': - case 'insert': { - const fileExists = await this.workspace.fs.exists(params.path) - if (!fileExists) { - throw new Error('The provided path must exist in order to replace or insert contents into it') - } - break - } - case 'append': - if (!params.path) { - throw new Error('Path must not be empty') - } - if (!params.newStr) { - throw new Error('Content to append must not be empty') - } - break - } - } - private async handleCreate(params: CreateParams, sanitizedPath: string): Promise { const content = this.getCreateCommandText(params) 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 610e4b8123..9aa41aa85b 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 @@ -6,9 +6,9 @@ export const FsToolsServer: Server = ({ workspace, logging, agent }) => { const fsReadTool = new FsRead({ workspace, logging }) const fsWriteTool = new FsWrite({ workspace, logging }) - agent.addTool(fsReadTool.getSpec(), (input: FsReadParams) => fsReadTool.invoke(input)) + agent.addTool(fsReadTool.getSpec(), fsReadTool.invoke) - agent.addTool(fsWriteTool.getSpec(), (input: FsWriteParams) => fsWriteTool.invoke(input)) + agent.addTool(fsWriteTool.getSpec(), fsWriteTool.invoke) return () => {} } From ceebf24ff46a32a9b243563c614cf0aa6c58d38d Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 1 Apr 2025 19:16:27 -0400 Subject: [PATCH 18/20] refactor: explicitly type input --- .../src/language-server/agenticChat/tools/toolServer.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 9aa41aa85b..610e4b8123 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 @@ -6,9 +6,9 @@ export const FsToolsServer: Server = ({ workspace, logging, agent }) => { const fsReadTool = new FsRead({ workspace, logging }) const fsWriteTool = new FsWrite({ workspace, logging }) - agent.addTool(fsReadTool.getSpec(), fsReadTool.invoke) + agent.addTool(fsReadTool.getSpec(), (input: FsReadParams) => fsReadTool.invoke(input)) - agent.addTool(fsWriteTool.getSpec(), fsWriteTool.invoke) + agent.addTool(fsWriteTool.getSpec(), (input: FsWriteParams) => fsWriteTool.invoke(input)) return () => {} } From af5c752eace304c6cf9db833490ad1230613b325 Mon Sep 17 00:00:00 2001 From: hkobew Date: Wed, 2 Apr 2025 09:33:43 -0400 Subject: [PATCH 19/20] refactor: make spec readonly --- .../src/language-server/agenticChat/tools/fsRead.ts | 2 +- .../src/language-server/agenticChat/tools/fsWrite.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts index 61addc8785..905c4134ef 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts @@ -112,6 +112,6 @@ export class FsRead { }, required: ['path'], }, - } + } as const } } diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts index 1845375e46..f938db8482 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts @@ -177,6 +177,6 @@ export class FsWrite { }, required: ['command', 'path'], }, - } + } as const } } From b3aa3510fb0018a5f90007508736a04a907ea39a Mon Sep 17 00:00:00 2001 From: hkobew Date: Wed, 2 Apr 2025 16:25:46 -0400 Subject: [PATCH 20/20] fix: avoid unnecessary type assertion --- .../src/language-server/agenticChat/tools/fsRead.ts | 2 +- .../src/language-server/agenticChat/tools/fsWrite.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts index 905c4134ef..3ab82df3c8 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts @@ -95,7 +95,7 @@ export class FsRead { description: 'A tool for reading a file. \n* This tool returns the contents of a file, and the optional `readRange` determines what range of lines will be read from the specified file.', inputSchema: { - type: 'object' as 'object', + type: 'object', parameters: { path: { description: 'Absolute path to a file, e.g. `/repo/file.py`.', diff --git a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts index f938db8482..cade189e53 100644 --- a/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts +++ b/server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsWrite.ts @@ -142,7 +142,7 @@ export class FsWrite { description: 'A tool for creating and editing a file.\n * The `create` command will override the file at `path` if it already exists as a file, and otherwise create a new file\n * The `append` command will add content to the end of an existing file, automatically adding a newline if the file does not end with one. The file must exist.\n Notes for using the `strReplace` command:\n * The `oldStr` parameter should match EXACTLY one or more consecutive lines from the original file. Be mindful of whitespaces!\n * If the `oldStr` parameter is not unique in the file, the replacement will not be performed. Make sure to include enough context in `oldStr` to make it unique\n * The `newStr` parameter should contain the edited lines that should replace the `oldStr`. The `insert` command will insert `newStr` after `insertLine` and place it on its own line.', inputSchema: { - type: 'object' as 'object', + type: 'object', parameters: { command: { type: 'string',