Fix: Specify sha is required for file update #320
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR updates the tool description for file operations to indicate that a SHA is required when updating a file.
- Updated the tool description for create_or_update_file.
- Clarified that the SHA field must be provided when updating a file.
| @@ -216,7 +216,7 @@ func ListBranches(getClient GetClientFn, t translations.TranslationHelperFunc) ( | |||
| // CreateOrUpdateFile creates a tool to create or update a file in a GitHub repository. | |||
| func CreateOrUpdateFile(getClient GetClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { | |||
| return mcp.NewTool("create_or_update_file", | |||
| mcp.WithDescription(t("TOOL_CREATE_OR_UPDATE_FILE_DESCRIPTION", "Create or update a single file in a GitHub repository")), | |||
| mcp.WithDescription(t("TOOL_CREATE_OR_UPDATE_FILE_DESCRIPTION", "Create or update a single file in a GitHub repository. If updating, you must provide the SHA of the file you want to update.")), | |||
There was a problem hiding this comment.
The updated tool description indicates that a SHA is required when updating a file, but there is no validation enforcing this constraint. Consider adding a check to ensure that the SHA field is provided when processing an update.
Copilot uses AI. Check for mistakes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes: #191
The problem
When the LLM sends an
update_filerequest, it doesn't include theshaof the file being replaced, leading to the error:This happens because
shais an optional field since the same tool is used for bothcreateandupdatefile operations.The solution
I did some local testing and the issue seems to be fixed if we specify in the tool description that
shais required when updating a file.However, a more robust solution might be to split the tool into 2 separate tools and and make a new
update_filetool that has a requiredshaproperty.I wasn't sure which approach would be preferred, so I've decided to put forward the simplest solution that works to hear your feedback.