fix(file_write_branch): support file creation by routing POST/PUT on sha #1

Merged
mathias merged 3 commits from fix/file-write-branch-create into main 2026-05-06 14:44:38 +00:00
Contributor

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).

## 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`).
gitea-mcp-bot added 2 commits 2026-05-06 14:05:49 +00:00
test(file_write_branch): cover POST-on-create and PUT-on-update routing
All checks were successful
CD / Lint / Test / Vet (pull_request) Successful in 6s
CD / Build & Import (pull_request) Has been skipped
CD / Deploy via GitOps (pull_request) Has been skipped
052827320a
Author
Contributor

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.

## 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.
gitea-mcp-bot added 1 commit 2026-05-06 14:35:22 +00:00
test(file_write_branch): assert branch and commit_sha on PUT path for parity
All checks were successful
CD / Lint / Test / Vet (pull_request) Successful in 5s
CD / Build & Import (pull_request) Has been skipped
CD / Deploy via GitOps (pull_request) Has been skipped
d35ff9781c
Author
Contributor

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 main 2026-05-06 14:44:38 +00:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: mathias/gitea-mcp#1