file_write_branch is documented as "Create or update a file on a feature branch", but creates fail. internal/gitea/files.go:UpsertFile unconditionally uses PUT /repos/{owner}/{repo}/contents/{path}, which Gitea routes to its update handler — and that handler requires sha. New files have no sha, so every create returns:
[SHA]: Required
This blocks adding any new tool to the connector via the connector itself, since every new tool needs a new *.go file in internal/tools/.
Fix
Route by HTTP method based on whether args.Sha is provided:
sha == "" → POST (create)
sha != "" → PUT (update)
Same URL, same payload shape (Sha already has omitempty), so no other call sites need to change. Keeps the public UpsertFile signature stable.
Tests
TestFileWriteBranchCreatesBranchAndFile — flipped to assert MethodPost on the contents endpoint (the test was previously baking in the buggy behavior).
TestFileWriteBranchUsesPutWhenShaProvided — new test exercising the update path with a sha.
Follow-up
Once this lands and the connector is redeployed, a follow-up PR can add issue_get (currently impossible to ship through the connector because it requires creating internal/tools/issue_get.go).
## Problem
`file_write_branch` is documented as "Create or update a file on a feature branch", but creates fail. `internal/gitea/files.go:UpsertFile` unconditionally uses `PUT /repos/{owner}/{repo}/contents/{path}`, which Gitea routes to its *update* handler — and that handler requires `sha`. New files have no sha, so every create returns:
[SHA]: Required
This blocks adding any new tool to the connector via the connector itself, since every new tool needs a new `*.go` file in `internal/tools/`.
## Fix
Route by HTTP method based on whether `args.Sha` is provided:
- `sha == ""` → `POST` (create)
- `sha != ""` → `PUT` (update)
Same URL, same payload shape (`Sha` already has `omitempty`), so no other call sites need to change. Keeps the public `UpsertFile` signature stable.
## Tests
- `TestFileWriteBranchCreatesBranchAndFile` — flipped to assert `MethodPost` on the contents endpoint (the test was previously baking in the buggy behavior).
- `TestFileWriteBranchUsesPutWhenShaProvided` — new test exercising the update path with a sha.
## Follow-up
Once this lands and the connector is redeployed, a follow-up PR can add `issue_get` (currently impossible to ship through the connector because it requires creating `internal/tools/issue_get.go`).
The UpsertFile routing logic is exactly right: POST when sha is empty (new file), PUT when sha is set (update). The comment on the function is accurate and useful. sha,omitempty on UpsertFileArgs is correct — it means sha is omitted from the JSON body on POST, which is what Gitea expects.
Existing file with SHA provided (PUT, no branch create)
Default base branch resolution when base is empty
Branch-already-exists skips POST to /branches
Allowlist rejection
Missing message validation
Use of atomic.Int32 to assert POST /branches is never called in TestFileWriteBranchSkipsCreateWhenBranchExists is a nice touch.
Minor note
TestFileWriteBranchUsesPutWhenShaProvided only checks require.NotNil(t, out). For consistency with TestFileWriteBranchCreatesBranchAndFile, it would be slightly better to assert commit_sha and branch are present in the output. Not a blocker.
## Review
**Verdict: ✅ Approve — one minor note**
### `internal/gitea/files.go` — solid
The `UpsertFile` routing logic is exactly right: `POST` when `sha` is empty (new file), `PUT` when `sha` is set (update). The comment on the function is accurate and useful. `sha,omitempty` on `UpsertFileArgs` is correct — it means `sha` is omitted from the JSON body on POST, which is what Gitea expects.
### `internal/tools/file_write_branch_test.go` — thorough
The test suite covers all the meaningful cases:
- New file on new branch (POST + branch create)
- Existing file with SHA provided (PUT, no branch create)
- Default base branch resolution when `base` is empty
- Branch-already-exists skips POST to `/branches`
- Allowlist rejection
- Missing `message` validation
Use of `atomic.Int32` to assert `POST /branches` is never called in `TestFileWriteBranchSkipsCreateWhenBranchExists` is a nice touch.
### Minor note
`TestFileWriteBranchUsesPutWhenShaProvided` only checks `require.NotNil(t, out)`. For consistency with `TestFileWriteBranchCreatesBranchAndFile`, it would be slightly better to assert `commit_sha` and `branch` are present in the output. Not a blocker.
Addressed in d35ff97 — TestFileWriteBranchUsesPutWhenShaProvided now parses out into a map and asserts branch == "feat/existing" and commit_sha == "cmt1", matching the create-test pattern.
Addressed in d35ff97 — TestFileWriteBranchUsesPutWhenShaProvided now parses out into a map and asserts branch == "feat/existing" and commit_sha == "cmt1", matching the create-test pattern.
mathias
merged commit 47e631da23 into main2026-05-06 14:44:38 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Problem
file_write_branchis documented as "Create or update a file on a feature branch", but creates fail.internal/gitea/files.go:UpsertFileunconditionally usesPUT /repos/{owner}/{repo}/contents/{path}, which Gitea routes to its update handler — and that handler requiressha. New files have no sha, so every create returns:This blocks adding any new tool to the connector via the connector itself, since every new tool needs a new
*.gofile ininternal/tools/.Fix
Route by HTTP method based on whether
args.Shais provided:sha == ""→POST(create)sha != ""→PUT(update)Same URL, same payload shape (
Shaalready hasomitempty), so no other call sites need to change. Keeps the publicUpsertFilesignature stable.Tests
TestFileWriteBranchCreatesBranchAndFile— flipped to assertMethodPoston the contents endpoint (the test was previously baking in the buggy behavior).TestFileWriteBranchUsesPutWhenShaProvided— new test exercising the update path with a sha.Follow-up
Once this lands and the connector is redeployed, a follow-up PR can add
issue_get(currently impossible to ship through the connector because it requires creatinginternal/tools/issue_get.go).Review
Verdict: ✅ Approve — one minor note
internal/gitea/files.go— solidThe
UpsertFilerouting logic is exactly right:POSTwhenshais empty (new file),PUTwhenshais set (update). The comment on the function is accurate and useful.sha,omitemptyonUpsertFileArgsis correct — it meansshais omitted from the JSON body on POST, which is what Gitea expects.internal/tools/file_write_branch_test.go— thoroughThe test suite covers all the meaningful cases:
baseis empty/branchesmessagevalidationUse of
atomic.Int32to assertPOST /branchesis never called inTestFileWriteBranchSkipsCreateWhenBranchExistsis a nice touch.Minor note
TestFileWriteBranchUsesPutWhenShaProvidedonly checksrequire.NotNil(t, out). For consistency withTestFileWriteBranchCreatesBranchAndFile, it would be slightly better to assertcommit_shaandbranchare present in the output. Not a blocker.Addressed in
d35ff97— TestFileWriteBranchUsesPutWhenShaProvided now parses out into a map and asserts branch == "feat/existing" and commit_sha == "cmt1", matching the create-test pattern.