Repository navigation
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 #78
Description
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
| 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: falsewith the upstream status.
What exists today
GitLfsCache/Locks/LockFanOut.cs:198-203: a non-success upstream answer becomesFailure(target.Path, id, response.StatusCode, Describe(body, ...)).Failure(LockFanOut.cs:102-122) writes onlyok,status,message,pathandid. The upstream body is read only to pull outmessage. OnlySuccess(LockFanOut.cs:277-306) copies upstream'slockobject through.LockFanOut.cs:209-213: when every attempt was throttled, the item is always reported asHttpStatusCode.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-103asserts onlystatus == 409on 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)
- When upstream's non-success body is a JSON object with a
lockproperty, the failure item includes it (deep-cloned, likeSuccessdoes), next tostatusandmessage. - An item that exhausts
MaxFanOutRetriesreports 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. - 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 carrieslock.owner.name. - Add a test: throttled with 403 +
Retry-Afteruntil retries run out, and the item reportsstatus: 403. - 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.
Activity
matt-edmondson commentedon Sep 29, 2026
Triage
GET locks?path=, at the cost of extra round trips. The 429-vs-403 status mismatch is cosmetic for most clients.GitLfsCache/Locks/LockFanOut.cs(Failure,SendWithRetriesAsync),LockFanOutTests, and the README fan-out example.Generated by Claude Code