Skip to content

Creating or unlocking a lock does not invalidate the cached lock list when the list was fetched with ?refspec= #46

Closed

Description

@matt-edmondson

What's wrong

LockRouteHandler.ListAsync caches lock lists under a key that includes the list request's refspec query parameter:

LockSnapshotKey key = new(route.Upstream, route.RepositoryPath,
    context.Request.Query["refspec"].FirstOrDefault());

After a relayed create (POST /locks) or unlock (POST /locks/:id/unlock), InvalidateIfChanged builds the key to clear from that request's query string:

lockSnapshots.Invalidate(new LockSnapshotKey(route.Upstream, route.RepositoryPath,
    context.Request.Query["refspec"].FirstOrDefault()));

In the Git LFS locking API, create and unlock carry the ref in the JSON body ({"path": ..., "ref": {"name": "refs/heads/main"}}), not in the query string. When the list was fetched with a refspec, it sits under (upstream, repo, "refs/heads/main"), but the change clears (upstream, repo, null), which is a different entry.

The batch fan-out path already gets this right: FanOutAsync builds its key from request.Ref in the body.

Failure scenario

  1. A git-lfs client lists locks with ?refspec=refs/heads/main, and the list is cached.
  2. The same user runs git lfs lock foo.psd. The upstream accepts the lock and the proxy clears the null-refspec key.
  3. git lfs locks is served the stale cached list without the new lock, for up to ListTtl. Unlocking has the same problem: a released lock keeps showing as held.

The existing LockListCachingTests.CreatingALock_InvalidatesTheSnapshot passes only because neither of its requests sends a refspec.

Suggested fix

Do one of the following:

  • In the create and unlock relay, buffer the request body, read ref.name, and invalidate that key.
  • Invalidate every cached snapshot for (upstream, repositoryPath) whatever the refspec. This is simpler and is safe, since invalidation only costs a refetch.

Acceptance criteria: a test lists with ?refspec=refs/heads/main, creates a lock whose body carries the same ref, and asserts that the next list includes the new lock.

Activity

  1. matt-edmondson commented on Sep 26, 2026

    @matt-edmondson
    ContributorAuthor

    Triage

    • Category: Bug
    • Priority: Medium. Clients see a stale lock list (new locks missing, released locks still shown) for up to ListTtl. The problem is self-healing and the upstream stays authoritative on lock enforcement, but it misleads git lfs locks users.
    • Area: Lock routes (LockRouteHandler.ListAsync / InvalidateIfChanged)
    • Suggested assignment: GitLfsCache maintainer, locking API area
    • Duplicates: None found. It is related in spirit to A disk-full error on the final chunk of a download publishes a truncated object that passes hash verification #45 (cache correctness) but is a separate bug.
    • In progress: No open PR covers this.

    Notes: Of the two suggested fixes, invalidating every snapshot for (upstream, repositoryPath) is the simpler and more robust choice. It doesn't need the body to be buffered, and it also covers clients that list with one refspec and lock with another. The existing CreatingALock_InvalidatesTheSnapshot test should gain a refspec-carrying variant.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

readyFully specified; implement as written

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions