Skip to content

Conversation

code-asher
Copy link
Member

A continuation from #19591

@code-asher code-asher force-pushed the asher/mcp-file-edit branch 2 times, most recently from f2ba812 to 5b5b443 Compare August 29, 2025 22:35
@code-asher code-asher changed the title Add coder_workspace_edit_file MCP tool feat: add coder_workspace_edit_file MCP tool Aug 29, 2025
@code-asher code-asher force-pushed the asher/mcp-file-edit branch 3 times, most recently from 91c875b to 9435706 Compare August 29, 2025 23:22
@github-actions github-actions bot added the stale This issue is like stale bread. label Sep 6, 2025
@Kira-Pilot Kira-Pilot requested a review from ThomasK33 September 8, 2025 18:23
@github-actions github-actions bot removed the stale This issue is like stale bread. label Sep 9, 2025
@code-asher code-asher force-pushed the asher/mcp-file-write branch 3 times, most recently from 8c658eb to 13e42f0 Compare September 9, 2025 23:25
Copy link
Member

@ThomasK33 ThomasK33 left a comment

Choose a reason for hiding this comment

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

I prefer this PR to be implemented as a multi-file edit (internally), and then have two MCP tools: one for a single file edit, and the other for bulk/multi-file edit operations.

Comment on lines +528 to +531
type FileEdit struct {
Search string `json:"search"`
Replace string `json:"replace"`
}
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to include a file path here as well?

This way, one could bundle multiple file writes or perform bulk operations into a single API call.

We can still have a tool that edits a single file, but internally, it could bundle the path and call it the multi-file edit one.

Comment on lines +232 to +234
if len(edits) == 0 {
return http.StatusBadRequest, xerrors.New("must specify at least one edit")
}
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this check be moved to the top?

Comment on lines +248 to +249
if err != nil {
return http.StatusInternalServerError, xerrors.Errorf("edit %s: %w", path, err)
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we clean up the tmpfile if the copy failed?

Base automatically changed from asher/mcp-file-write to main September 11, 2025 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants