Skip to content

chore: sync fsRead with VSC branch. #936

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Hweinstock merged 4 commits into from
Apr 11, 2025
Merged

chore: sync fsRead with VSC branch. #936

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9dcf688
chore: sync fsRead tool with recent changes
Hweinstock Apr 9, 2025
6791fec
docs: add todo for likely next steps
Hweinstock Apr 9, 2025
769566b
test: add a test for stream usage
Hweinstock Apr 9, 2025
7f5dca2
chore: update spec formatting to line up
Hweinstock Apr 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 30 additions & 23 deletions server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,24 @@ describe('FsRead Tool', () => {
tempFolder.clear()
})

it('throws if path is empty', async () => {
it('invalidates empty path', async () => {
const fsRead = new FsRead(features)
await assert.rejects(() => fsRead.invoke({ path: '' }))
await assert.rejects(
fsRead.validate({ path: '' }),
/Path cannot be empty/i,
'Expected an error about empty path'
)
})

it('invalidates non-existent paths', async () => {
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'
)
})

it('reads entire file', async () => {
Expand All @@ -70,27 +85,6 @@ describe('FsRead Tool', () => {
assert.strictEqual(result.output.content, 'B\nC\nD')
})

it('throws error if path does not exist', async () => {
const filePath = path.join(tempFolder.path, 'no_such_file.txt')
const fsRead = new FsRead(features)

await assert.rejects(fsRead.invoke({ path: filePath }))
})

it('throws error if content exceeds 30KB', async function () {
const bigContent = 'x'.repeat(35_000)

const filePath = await tempFolder.write('bigFile.txt', bigContent)

const fsRead = new FsRead(features)

await assert.rejects(
fsRead.invoke({ path: filePath }),
/This tool only supports reading \d+ bytes at a time/i,
'Expected a size-limit error'
)
})

it('invalid line range', async () => {
const filePath = await tempFolder.write('rangeTest.txt', '1\n2\n3')
const fsRead = new FsRead(features)
Expand All @@ -100,4 +94,17 @@ describe('FsRead Tool', () => {
assert.strictEqual(result.output.kind, 'text')
assert.strictEqual(result.output.content, '')
})

it('updates the stream', async () => {
const fsRead = new FsRead(features)
const chunks = []
const stream = new WritableStream({
write: c => {
chunks.push(c)
},
})
await fsRead.queueDescription({ path: 'this/is/my/path' }, stream)
assert.ok(chunks.length > 0)
assert.ok(!stream.locked)
})
})
62 changes: 41 additions & 21 deletions server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/fsRead.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
/*!
* 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 { InvokeOutput } from './toolShared'
import { Features } from '@aws/language-server-runtimes/server-interface/server'

// Port of https://github.com/aws/aws-toolkit-vscode/blob/10bb1c7dc45f128df14d749d95905c0e9b808096/packages/core/src/codewhispererChat/tools/fsRead.ts#L17
// Port of https://github.com/aws/aws-toolkit-vscode/blob/8e00eefa33f4eee99eed162582c32c270e9e798e/packages/core/src/codewhispererChat/tools/fsRead.ts#L17

export interface FsReadParams {
path: string
Expand All @@ -22,6 +18,41 @@ export class FsRead {
this.workspace = features.workspace
}

public async validate(params: FsReadParams): Promise<void> {
this.logging.debug(`Validating path: ${params.path}`)
if (!params.path || params.path.trim().length === 0) {
throw new Error('Path cannot be empty.')
}

const fileExists = await this.workspace.fs.exists(params.path)
if (!fileExists) {
throw new Error(`Path: "${params.path}" does not exist or cannot be accessed.`)
}

this.logging.debug(`Validation succeeded for path: ${params.path}`)
}

public async queueDescription(params: FsReadParams, updates: WritableStream) {
const updateWriter = updates.getWriter()
await updateWriter.write(`Reading file: ${params.path}]`)

const [start, end] = params.readRange ?? []

if (start && end) {
await updateWriter.write(`from line ${start} to ${end}`)
} else if (start) {
if (start > 0) {
await updateWriter.write(`from line ${start} to end of file`)
} else {
await updateWriter.write(`${start} line from the end of file to end of file`)
}
} else {
await updateWriter.write('all lines')
}
await updateWriter.close()
updateWriter.releaseLock()
}

public async invoke(params: FsReadParams): Promise<InvokeOutput> {
const path = sanitize(params.path)
const fileContents = await this.readFile(path)
Expand All @@ -37,7 +68,7 @@ export class FsRead {
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))
return this.createOutput(fullText)
}

const lines = fullText.split('\n')
Expand All @@ -49,7 +80,7 @@ export class FsRead {

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))
return this.createOutput(slice)
}

private parseLineRange(lineCount: number, range: number[]): [number, number] {
Expand All @@ -69,17 +100,6 @@ export class FsRead {
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: {
Expand All @@ -93,7 +113,7 @@ export class FsRead {
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.',
'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',
properties: {
Expand All @@ -103,7 +123,7 @@ export class FsRead {
},
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.',
'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.',
type: 'array',
items: {
type: 'number',
Expand Down
7 changes: 6 additions & 1 deletion server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolServer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@ export const FsToolsServer: Server = ({ workspace, logging, agent }) => {

const listDirectoryTool = new ListDirectory({ workspace, logging })

agent.addTool(fsReadTool.getSpec(), (input: FsReadParams) => fsReadTool.invoke(input))
agent.addTool(fsReadTool.getSpec(), async (input: FsReadParams) => {
// TODO: fill in logic for handling invalid tool invocations
// TODO: implement chat streaming via queueDescription.
await fsReadTool.validate(input)
Copy link
Contributor

@kmile kmile Apr 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will throw errors to be handled by the chatController (which I think is fine, but good to be aware of).

Copy link
Contributor Author

@Hweinstock Hweinstock Apr 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I wasn't sure if this should be handled in the handler or in the chatController itself which is why I added the comment.

However, I imagine handling these agent errors in one place and allowing handlers to throw is better design, so good call out!

await fsReadTool.invoke(input)
})

agent.addTool(fsWriteTool.getSpec(), (input: FsWriteParams) => fsWriteTool.invoke(input))

Expand Down
2 changes: 0 additions & 2 deletions server/aws-lsp-codewhisperer/src/language-server/agenticChat/tools/toolShared.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
export const maxToolResponseSize = 30720 // 30KB

export interface InvokeOutput {
output:
| {
Expand Down
Loading