feat: initial fsWrite chat message #1026
Conversation
| switch (toolUse.name) { | ||
| case 'fsRead': | ||
| case 'listDirectory': | ||
| // no need to write tool result for listDir and fsRead into chat stream |
There was a problem hiding this comment.
nit: Can I know the reason, Why ? May be I am missing some context here!
There was a problem hiding this comment.
we are sending it before running the tool above, but actually it might make more sense to move it here
There was a problem hiding this comment.
We should send the tool use with the list of paths before we run it, so we tell the user what we're doing before we're actually doing it.
I can see us update a spinner to a checkmark here upon completion.
But we don't have to repeat the tool output (the list of files, the status of the edit) unless there is an error.
| const fileName = path.basename(input.path) | ||
| // TODO: right now diff changes is coupled with fsWrite class, we should move it to shared utils | ||
| const fsWrite = new FsWrite(this.#features) | ||
| const diffChanges = await fsWrite.getDiffChanges(input) |
There was a problem hiding this comment.
This could be a static, but even better would be in a shared util like you mentioned so the representation is not coupled with the tool itself.
| header: { | ||
| ...header, | ||
| // TODO: this is specific to fsWrite tool, modify for other tools as needed | ||
| fileList: { ...header?.fileList, fileTreeTitle: '', hideFileCount: true }, |
There was a problem hiding this comment.
is only this line going to be modified for other tools or all of it? If so, just wondering if it makes sense to pull this out to a different file so that its easy to understand which tools have which interface
There was a problem hiding this comment.
+1 to separating this logic out.
| header: { | ||
| ...header, | ||
| // TODO: this is specific to fsWrite tool, modify for other tools as needed | ||
| fileList: { ...header?.fileList, fileTreeTitle: '', hideFileCount: true }, |
There was a problem hiding this comment.
+1 to separating this logic out.
| // no need to write tool result for listDir and fsRead into chat stream | ||
| if (toolUse.name !== 'fsRead' && toolUse.name !== 'listDirectory') { | ||
| await chatResultStream.writeResultBlock({ body: toolResultMessage(toolUse, result) }) | ||
| switch (toolUse.name) { |
There was a problem hiding this comment.
For the sake of future MCP support, I think this tool specific branching should be isolated from the core logic. This also won't scale well as we add more tools.
Problem
Initial FsWrite tool message implementation
Solution
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.