feat(repo-server): patch multiple files in one atomic commit

[Vogeslu]

What

Adds PUT /api/v1/patches, which patches any number of files in one request and
commits all of them as a single commit. Previously a caller had to send one
request per file and got one commit per file, with no atomicity across files.

PUT /api/v1/patch is unchanged and stays wire compatible.

API

{
  "actor": "ci-bot",
  "files": [
    { "filePath": "applications/dev/service-foo/values.yaml",
      "patches": [{ "selector": ".service.image.tag", "value": "v42.0.1" }] },
    { "filePath": "applications/dev/service-bar/values.yaml",
      "patches": [{ "selector": ".service.image.tag", "value": "v42.0.1" }] }
  ]
}

Response 200 {"message":"ok","commit":"<sha>"}. The commit id is empty when the
batch did not change anything. actor is optional and becomes a Triggered by:
footer on the shared commit message.

Atomicity

All files are read and patched in memory before anything is written, so a missing
file, an unreadable file, a symlink, an untracked file or a selector that does not
match fails the whole request with no file touched.

Every failure after that point, a write error, a failed commit, a rebase conflict or
a push that keeps failing, runs one recovery: git rebase --abort (if a rebase is
in progress) followed by git reset --hard origin/<branch>. The request answers
500, nothing stays half written, staged or committed but unpushed, and the caller
retries against fresh state. A commit whose push fails is discarded rather than kept
local: a kept commit that later conflicts with an upstream change would leave the
long-running clone in an active rebase that nothing clears, taking both endpoints
down until the pod restarts.

The returned commit id is HEAD after the push, so it is the rebased commit when
pull --rebase had to move the batch on top of a concurrent change.

Validation

Rejected with 400 and a message naming the offending index, before the git lock
is taken so invalid requests do not block concurrent patches:

• empty files, a file without patches, an empty selector
• duplicate file paths (compared after cleaning)
• absolute paths, paths escaping the repository, paths starting with - or :

A leading dash would be parsed as a git option, a leading colon as pathspec magic
(:!*.yaml). git add additionally gets a -- separator. The checks run on the
cleaned path, so ./-n.yaml is rejected too. The Go client runs the same validation
before sending a request.

Inside the git lock, a target that is not a regular file (Lstat, so symlinks are
seen as such and cannot redirect the write inside or outside the repository) is
rejected with 400. A target that is not tracked by git fails with 500, because
it would be written but never committed.

Incidental fixes

GitPatcher.Patch behind PUT /api/v1/patch now runs through PatchBatch, so
the single file endpoint gets the same in-memory prepare, the same recovery and the
same 400 for invalid input (previously 500). This also removes two pre-existing
bugs in the old implementation:

• a failing os.Stat was only logged, causing a nil pointer dereference on a
  file that does not exist (and, with no gin.Recovery(), a dropped connection)
• the "no changes" branch returned from inside the task loop, skipping every
  remaining task and the push

Notable internals

• the patch lock moves from the handler closure onto Server, so both patch
  endpoints serialize their git operations against each other
• Server.GitPatcher becomes the patch.PatchMethod interface, which makes the
  handlers testable without a git repository
• new internal/git helpers: ResetToUpstream, RequireTracked, RevParse,
  CommitFiles
• RepositoryServerPatcher gains a matching PatchBatch and no longer discards
  everything past the first task

Tests

31 new test functions, ~1000 lines of test code.

• table driven validation and path cleaning tests, no git required
• the first HTTP handler tests in this repo, using httptest and a fake
  PatchMethod: success, empty commit, each validation error, patcher error,
  missing and wrong API key, plus regressions that /patch and /health still
  behave as before
• client tests against httptest.NewServer asserting method, path, headers and
  that every file is sent
• soft-serve integration tests proving both files land in one commit
  (CommitFiles(sha)), that the commit exists in an independent clone (checked
  with git show, since rev-parse echoes any 40-hex string with exit 0), that a
  missing file, a bad selector, a symlink, an untracked file or a failing write all
  leave the working tree clean, that a pending commit conflicting with upstream
  leaves a clean and usable clone behind, that the returned id is the rebased
  commit when upstream moved before the push, and that ResetToUpstream clears
  local commits, staged changes and an active rebase

Verified end to end against the repo-server running in Docker against soft-serve:
one commit for a two file batch, empty commit id on replay, 400 on a traversing
path, 500 with nothing changed on a missing file, and the old endpoint unaffected.

Note for CI: internal/git and internal/patch push to the same soft-serve repo
and go test ./internal/... runs them concurrently. This is pre-existing, but the
added integration tests push more often. If TestGitPullFastForward starts
flaking with "fetch first", running the tests with -p 1 fixes it. Tests in
internal/git must not modify the shared fixture file
applications/dev/service-test/values.yaml, TestGitSshPatch depends on it.

Review

b0de979 addresses a dual review (Claude Fable 5.1 and Codex gpt-6-astra, both
high effort), see the review comment below for the finding-by-fix table.

Out of scope

• multi-file grammar for the gitops patch CLI, which keeps its single file form
• a custom commit message field in the request
• unrelated pre-existing issues: no gin.Recovery(), non constant time API key
  comparison, missing return after the empty key abort in the auth middleware,
  the patch lock being held through the push backoff sleeps, HasChanges checking
  the whole tree instead of the batch's files

[mxcd]

Dual review: Claude Fable 5.1 (high) + Codex gpt-6-astra (high)

Both reviewers independently returned DO NOT MERGE on a31f22e. Fixed in b0de979.

#	Finding	Fable	Codex	Fix
1	Pending local commit conflicts with a later upstream change: pull --rebase leaves an active rebase nobody clears, both endpoints dead until pod restart	blocker, reproduced	major	pending-commit retry removed; any failure runs rebase --abort + reset --hard origin/<branch> and returns 500
2	Symlink target written through, commit fails, target stays dirty, all later pulls refuse	blocker, reproduced	major	Lstat + regular-file check, 400
3	Failed scoped checkout HEAD -- restore only logged; fails wholesale if any path is untracked	major	major	same hard reset as 1
4	Returned commit id taken before pull --rebase, stale hash after rebase	major, reproduced	major	HEAD resolved after the push
5	PUT /patch had no rollback at all, shares the clone	major	-	Patch routes through PatchBatch; validation errors now 400 like /patches
6	Untracked target patched on disk, 200 with empty commit	minor	major	all targets must be tracked before any write
7	RevParse(hash) test assertion passes for any 40-hex string	minor	minor	verification clone uses git show
8	Pathspec magic (:!x.yaml) survives validation, git add without --	nit	major	-- added, leading : rejected

Not addressed, pre-existing: lock held through push backoff sleeps, HasChanges is whole-tree.

Design change worth noting: the "commit stays local and is pushed on retry" behaviour from the PR description is gone. A failed push now discards the commit and the caller retries. Both reviewers recommended this; the retry design is what made finding 1 reachable.

Verified with go test -p 1 ./... against soft-serve, all green. Full review text: both reports were kept locally, ask if you want them attached.

[mxcd]

PR description updated to match b0de979. Sections that changed:

• Atomicity now describes the single recovery path (rebase --abort + reset --hard origin/<branch>, 500, caller retries) and states explicitly that a commit whose push fails is discarded instead of kept local, with the reason.
• Validation adds the leading : rejection, the -- separator on git add, the regular-file check (Lstat, 400) and the tracked check (500).
• Incidental fixes notes that PUT /patch runs through PatchBatch and now answers 400 for invalid input instead of 500.
• Notable internals lists ResetToUpstream and RequireTracked in place of Restore.
• Tests lists the added integration cases and the git show based commit assertion.
• Out of scope adds the lock held through backoff sleeps and the whole-tree HasChanges, both pre-existing.

CI on this PR is waiting for a manual workflow approval.