fix(repos): give create_or_update_file callers a SHA they can actually get

[dylanpulver]

Summary

create_or_update_file told its caller to run git rev-parse <branch>:<path> to get a blob SHA, which an MCP client has no way to do. This replaces that instruction with the SHA itself where the server has it, and with a pointer to get_file_contents where it does not.

Why

Fixes#3130

The tool description says: "Use this tool to create or update a file in a GitHub repository remotely; do not use it for local file operations." Two lines later it says:

In order to obtain the SHA of original file version before updating, use the following git command:
git rev-parse <branch>:<path to file>

git rev-parse <branch>:<path> reads a local object database. A client that is using this tool over the API has no clone and no working tree, so it cannot run that. The same instruction appears in both SHA error paths, so an agent that hits either one is handed a recovery step it cannot perform, and it gets stuck in the loop described in #3130.

The already-exists path is the sharper case. Before returning that error the handler calls client.Repositories.GetContents and holds the result in existingFile. The SHA the caller is being sent away to fetch is in scope at that point.

What changed

• Tool description now points at get_file_contents, which returns the blob SHA over the API, instead of git rev-parse.
• The already-exists error returns existingFile.GetSHA() so the caller can retry immediately with no extra round trip.
• The stale-SHA error drops the git rev-parse sentence. It interpolates the current SHA, so nothing else was needed there.
• Updated pkg/github/__toolsnaps__/create_or_update_file.snap for the description change.
• Updated Test_CreateOrUpdateFile, plus an assertion in the shared error branch that no error path in this tool emits git rev-parse.

Error strings before:

SHA mismatch: provided SHA %s is stale. Current file SHA is %s. Pull the latest changes and use git rev-parse %s:%s to get the current SHA.
File already exists at %s. You must provide the current file's SHA when updating. Use git rev-parse %s:%s to get the blob SHA, then retry with the sha parameter.

After:

SHA mismatch: provided SHA %s is stale. Current file SHA is %s. Re-read the file with get_file_contents if you need its latest content, then retry with the sha parameter set to %s.
File already exists at %s. You must provide the current file's SHA when updating. The current SHA is %s; retry with the sha parameter set to that value.

I grepped the whole package for other local-git instructions in tool-facing strings. The three sites fixed here were the only hits outside Dockerfile and script/.

Relationship to #2772

#2772 rewrites create_or_update_file to commit through the GraphQL createCommitOnBranch mutation. It keeps the SHA validation block, and it does not modify the description or either error string, so the two changes touch different lines of the same function.

I checked this rather than assumed it. Cherry-picking this commit onto pull/2772/head applies pkg/github/repositories.go and the toolsnap with no conflict. pkg/github/repositories_test.go conflicts in two table entries, but only because #2772 branched from a June main that predates the expectedErrMsgs field and the errorText builder in the shared assertion block. #2772 currently reports as conflicting with main and will need that same rebase regardless of this PR.

Landing this separately seems worth it because the bad instruction is shipping today while #2772 is a large architectural change with an uncertain timeline. The fix here is small enough to review on its own. If #2772 lands first I am happy to rebase this on top of it.

MCP impact

• Tool schema or behavior changed

The create_or_update_file description changed, and its two SHA error messages changed. Inputs, outputs, and success behavior are untouched. The already-exists error now carries a blob SHA, which the caller had read access to anyway, since the server obtained it with the caller's own credentials on the request that produced the error.

Prompts tested (tool changes only)

Read against the mocked handler paths in Test_CreateOrUpdateFile rather than a live token, since I do not have an e2e PAT for this repo. The two error paths correspond to these agent-facing cases:

• "Update docs/example.md on main to say X" where the file exists and the agent does not supply a sha. Previously returned the git rev-parse instruction; now returns the current blob SHA to retry with.
• "Update docs/example.md on main" with a sha captured before someone else pushed to the branch. Previously returned the git rev-parse instruction alongside the current SHA; now returns the current SHA and a retry instruction.

Security / limits

• Data exposure, filtering, or token/size limits considered

The already-exists error surfaces a blob SHA the server just read using the caller's own credentials on that same call. A caller that could not read the file would have received the GetContents error instead.

Tool renaming

• I am renaming tools as part of this PR (e.g. a part of a consolidation effort)
  • I have added the new tool aliases in deprecated_tool_aliases.go
• I am not renaming tools as part of this PR

Lint & tests

• Linted locally with ./script/lint
• Tested locally with ./script/test

./script/lint reports 0 issues../script/test passes across all packages. go vet ./... is clean. Both were run on a clean main checkout first for a baseline, and both were green there too.

I also confirmed the new assertions fail against unmodified repositories.go, so they are testing the fix rather than passing vacuously.

Not run: the e2e/ suite, which needs GITHUB_MCP_SERVER_E2E_TOKEN.

Docs

• Not needed

script/generate-docs produces no diff. README lists the tool title and parameters, not the description body.

[SamMorrowDrums]

Hi, so the reason this protection exists is so that the model is updating a version of a file that it is expecting to update.

Race conditions occur where two users update a file at same time and second one undoes the work. Just returning the head sha of the file is risky. I'd it wasn't we'd just update the file and not error.

Get file content returns the SHA so the model can use it.

I'm ok with an error that doesn't assume git access, but the model needs to know what version of a file it's actually editing and refetch it if it's changed since.

FWIW I see you do cover this, and I will hopefully look fully soon.