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

git rev-parse reads a local object database, and a client using this tool over the API has no clone. The instruction appears in both SHA error paths, so an agent hitting either is handed a recovery step it cannot perform. On the already-exists path the handler has already called client.Repositories.GetContents and is holding existingFile, so the SHA the caller is being sent away to fetch is in scope.

What changed

• Description points at get_file_contents instead of git rev-parse.
• The already-exists error returns existingFile.GetSHA(), so the caller can retry with no extra round trip.
• The stale-SHA error drops the git rev-parse sentence; it already interpolates the current SHA.
• Updated create_or_update_file.snap and Test_CreateOrUpdateFile, including an assertion that no error path in this tool emits git rev-parse.

I grepped the package for other local-git instructions in tool-facing strings; these three were the only hits outside Dockerfile and script/.

#2772 rewrites this function to commit through createCommitOnBranch, keeps the SHA validation block, and touches neither the description nor either error string. Cherry-picking this onto pull/2772/head applies cleanly except for two test-table entries, and only because #2772 branched from a June main predating the expectedErrMsgs field; it reports as conflicting with main today and needs that rebase regardless. Happy to rebase on top of it if it lands first.

MCP impact

• Tool schema or behavior changed

Description and the two SHA error messages changed. Inputs, outputs and success behavior are untouched.

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.

• "Update docs/example.md on main to say X" where the file exists and no sha is supplied: now returns the current blob SHA to retry with.
• The same with a sha captured before someone else pushed: 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 with the caller's own credentials on that same call. A caller who 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 ./... clean, all green on a clean main baseline too. The new assertions fail against unmodified repositories.go. Not run: e2e/, 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.

[dylanpulver]

There are two error paths here and only one of them loosens anything. Splitting them:

Stale SHA, where the caller did supply a sha: main already interpolated the current SHA into that message ("Current file SHA is %s") before this PR, and it still does. The only thing this PR changed on that path was the recovery step, from git rev-parse to get_file_contents. No new information is disclosed.

File exists and no sha was supplied: your objection lands. A caller on that path has not read the file, and handing it the SHA lets it overwrite content it never saw in one retry. That is a real weakening of the gate and I have taken it back out. The message now sends the caller to get_file_contents for the path and ref and tells it to retry with the SHA that call reports, so obtaining the SHA still requires reading the file, which is what the gate was buying. Same protection as main, without the local-git instruction that #3130 is about.

I also made the stale-SHA recovery imperative instead of conditional. It read "re-read the file with get_file_contents if you need its latest content"; the file definitely changed, so the re-read is not optional. It now says to re-read, rebuild the content against what comes back, and retry with the SHA that call reports.

Test side: added an unexpectedErrMsgs field to the Test_CreateOrUpdateFile table and asserted the already-exists error does not contain the blob SHA, so the gate cannot be loosened again without a test going red. The existing assertion that no error path emits git rev-parse stays. go test ./... passes, go vet ./... is clean, and create_or_update_file.snap is unchanged since the tool description did not move again.