`locks/batch` failure results drop upstream's `lock` object, so a 409 conflict no longer says who holds the file, and exhausted retries always report 429

[matt-edmondson]

Plan reference

GitLfsCache/docs/superpowers/specs/2026-08-19-locks-subsystem-design.md

Lines 113 to 121 in 7fc4362

	`operation` is `lock` or `unlock`. `unlock` accepts `ids` or `paths`, and `force` as a boolean. Response is always 200 when the request itself was well formed and the caller was admitted, because partial success is the normal outcome and a transport-level failure would discard the successful half:
	```json
	{
	"results": [
	{ "path": "Content/A.uasset", "ok": true, "lock": { "id": "871", "path": "Content/A.uasset", "locked_at": "2026-08-19T09:47:00Z", "owner": { "name": "someone" } } },
	{ "path": "Content/B.uasset", "ok": false, "status": 409, "message": "already locked", "lock": { "owner": { "name": "someone else" } } }
	]
	}

Response is always 200 when the request itself was well formed ...

{ "path": "Content/B.uasset", "ok": false, "status": 409, "message": "already locked", "lock": { "owner": { "name": "someone else" } } }

and

GitLfsCache/docs/superpowers/specs/2026-08-19-locks-subsystem-design.md

Line 132 in 7fc4362

GitHub secondary rate limits and Azure DevOps throttling are the expected failure, not an edge case. A 429 or 403 carrying `Retry-After` pauses the limiter for the whole upstream for that duration and the item is retried, up to `Locks:MaxFanOutRetries`. Items exhausting retries return `ok: false` with the upstream status. This behaviour is load bearing and belongs in the first implementation, not a follow-up.

Items exhausting retries return ok: false with the upstream status.

What exists today

• GitLfsCache/Locks/LockFanOut.cs:198-203: a non-success upstream answer becomes Failure(target.Path, id, response.StatusCode, Describe(body, ...)). Failure (LockFanOut.cs:102-122) writes only ok, status, message, path and id. The upstream body is read only to pull out message. Only Success (LockFanOut.cs:277-306) copies upstream's lock object through.
• LockFanOut.cs:209-213: when every attempt was throttled, the item is always reported as HttpStatusCode.TooManyRequests (429), even when upstream said 403 + Retry-After, which is GitHub's secondary-rate-limit shape (LockFanOut.cs:330-333).
• README's example (README.md L195) shows the failure item without lock, so the docs now describe the as-built shape. The spec's As built section records no decision to drop it.
• GitLfsCache.Tests/Integration/LockFanOutTests.cs:97-103 asserts only status == 409 on a conflict.

What's missing / why it matters

The Git LFS locking API answers a create conflict with 409 and a body carrying the existing lock (id, path, owner). That tells git-lfs, and the Unreal plugin this subsystem was built for, who holds the file. Through locks/batch that information is dropped. A client locking 500 assets gets "already locked" for each conflict and has to issue extra GET locks?path= calls to find the owner. That puts back round trips the fan-out exists to remove, and the listing it reads can be up to ListTtl stale.

Acceptance criteria (from the spec)

1. When upstream's non-success body is a JSON object with a lock property, the failure item includes it (deep-cloned, like Success does), next to status and message.
2. An item that exhausts MaxFanOutRetries reports the status upstream last returned (e.g. 403 for a GitHub secondary rate limit), not a hard-coded 429. Keep the "upstream kept throttling this call" message.
3. Add a test in LockFanOutTests: the stub upstream answers 409 with {"lock":{"id":"1","path":"Content/B.uasset","owner":{"name":"someone else"}},"message":"already created lock"}, and the result item carries lock.owner.name.
4. Add a test: throttled with 403 + Retry-After until retries run out, and the item reports status: 403.
5. Update the README fan-out example to match.

Dependencies

None. It is independent of #71 (unlock-by-path retry) and #74 (transport errors), though all three touch LockFanOut.SendWithRetriesAsync, so whichever lands second may need a rebase.

[matt-edmondson]

Triage

• Category: Improvement (the build doesn't match the locks spec)
• Priority: Low. The results are correct, but they carry less information than upstream sends. Clients that need the lock owner can call GET locks?path=, at the cost of extra round trips. The 429-vs-403 status mismatch is cosmetic for most clients.
• Area / suggested assignment: GitLfsCache/Locks/LockFanOut.cs (Failure, SendWithRetriesAsync), LockFanOutTests, and the README fan-out example.
• Duplicates: none. It touches the same method as Unlock by path fails with 404/409 when the snapshot names a released-and-reissued lock id: the spec's one refresh + one retry isn't implemented #71 and One network error on a single locks/batch item turns the whole fan-out into a 500, discarding locks upstream already granted #74, so expect a rebase between them.
• In progress: no matching open PR. The only open PRs in the repo are Dependabot bumps (Bump Testably.Abstractions.FileSystem.Interface from 10.3.0 to 10.4.0 #53, Bump Testably.Abstractions.Testing from 7.0.2 to 7.1.0 #54).

---

Generated by Claude Code