fix(gitrepo): support reftable ref storage#1723
Open
entire[bot] wants to merge 7 commits into
Open
Conversation
Opening a repository that uses the reftable ref backend (git init
--ref-format=reftable, or git refs migrate --ref-format=reftable) failed
with "core.repositoryformatversion does not support extension:
refstorage", so `entire enable` and every capture operation aborted.
Root cause: the vendored go-git v6 alpha has no reftable reader. Its
filesystem storer reads refs from .git/refs, .git/packed-refs and
.git/HEAD (a reftable repo's HEAD is a "ref: refs/heads/.invalid" stub
and the real refs live in the binary reftable stack), and its extension
verification rejects extensions.refstorage=reftable outright. Upstream
reftable support has not landed: go-git PR #1970 was closed unmerged and
go-git issue #1827 is still open, so we cannot wait for the dependency.
Fix: detect reftable repositories (presence of a reftable/ directory
under the worktree or common git dir) in gitrepo.OpenPath and wrap
go-git's filesystem storage with a reftableStorer. Object, config, index
and shallow storage keep flowing through go-git (reftable changes only
ref storage, not object storage); the ReferenceStorer methods
(SetReference, CheckAndSetReference, Reference, IterReferences,
RemoveReference, CountLooseRefs, PackRefs) are overridden to shell out
to git plumbing (update-ref, symbolic-ref, rev-parse, for-each-ref),
and the storer advertises SupportsExtension("refstorage") so go-git's
open-time extension check passes. This extends the existing "prefer the
git CLI where go-git is unsafe" seam already used for reset/checkout
(HardResetWithProtection) and fetch rather than inventing a new
architecture, and routes every ref read/write through a single point so
all callers (enable, session start, shadow branches, condensation, the
checkpoints branch, checkpoint list) work unchanged. Ref plumbing does
not fire git hooks, so this cannot recurse back into entire.
Verified end to end against reftable repos: enable, session start, file
changes, stop, user commit with Entire-Checkpoint trailer, checkpoint
list, and a linked worktree of a reftable repo, with no regression on
the classic files backend or sha256 object format.
Fixes #547
suhaanthayyil
changed the title
suhaanthayyil
marked this pull request as ready for review
July 12, 2026 01:05
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds support for Git repositories using the reftable ref backend by detecting reftable storage at repo-open time and routing ref reads/writes through the git CLI while keeping object storage on go-git. This unblocks entire enable and the full capture lifecycle on reftable repos (issue #547).
Changes:
- Detect reftable repositories in
gitrepo.OpenPathand wrap go-git storage with areftableStorer. - Implement a
reftableStorerthat overrides go-git ref operations usinggitplumbing commands (update-ref,symbolic-ref,rev-parse,for-each-ref). - Add unit and integration coverage for enable → checkpoint flow (including linked worktrees) on reftable repos.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| cmd/entire/cli/gitrepo/repository.go | Detects reftable repos and opens them with a ref-CLI-backed storer wrapper. |
| cmd/entire/cli/gitrepo/reftable.go | New reftable-aware storer that shells out to git for ref CRUD/iteration and extension support. |
| cmd/entire/cli/gitrepo/reftable_test.go | Unit tests for opening reftable repos and verifying ref read/write/list/remove behavior. |
| cmd/entire/cli/integration_test/reftable_repo_test.go | Integration tests covering end-to-end capture flow + linked worktree behavior on reftable repos. |
The reftable ReferenceStorer mapped every git update-ref failure to
storage.ErrReferenceHasChanged. strategy.atomicSetV1Ref keys a
privacy-critical pre-push decision on that sentinel: it treats it as
"another worktree advanced the ref" and aborts the push as a concurrency
event, while wrapping every other error as a real failure. The coarse
mapping therefore misreported storage errors (nonexistent object, invalid
ref name, lock contention, timeout, git spawn failure) as concurrency
conflicts, producing a bogus V1RefMovedError ("ref moved from X to X").
Map only a genuine compare-and-swap conflict ("is at X but expected Y",
or "reference already exists" for a create race) to
ErrReferenceHasChanged; surface every other failure as itself.
Also pass user-controlled ref names after --end-of-options so a ref name
beginning with "-" is treated as a literal argument rather than a git
option (defense-in-depth; args were already argv, never a shell string,
so there was no command-injection exposure).
Add mutation-verified regression tests: the CAS conflict-vs-error
mapping, and an argv-safety test proving ref names are never
shell-evaluated.
Reference, IterReferences (HEAD resolution), and RemoveReference treated every git failure as a definitive negative: a spawn failure, our timeout, or an I/O error was reported as ErrReferenceNotFound, a silently-dropped HEAD, or a successful deletion. A transient git failure masquerading as "ref absent" can make the strategy treat a live checkpoint ref as gone (orphan reset, lost linkage) or believe a ref was deleted when it was not. Classify failures via refLookupAbsent: git genuinely reports absence only when it ran and exited non-zero with empty stderr (its --quiet not-found contract). A wrapped context error (execGit now re-wraps the timeout kill), a non-*exec.ExitError (spawn/I/O), or a non-zero exit carrying stderr is surfaced as a real error instead. RemoveReference no longer treats an empty-stderr failure as idempotent success — git exits 0 for an already-absent ref, so a non-zero exit here is a genuine failure. Add an injectable runGitFn so the classification is covered by mutation-verified unit tests: absence, timeout, spawn failure, and non-zero-with-stderr for Reference; empty-stderr and timeout not swallowed for RemoveReference; and a git failure resolving HEAD surfaced by IterReferences.
…eference RemoveReference probes symbolic-ref -q to choose between deleting via symbolic-ref -d (symbolic) or update-ref -d (hash). It treated ANY probe error as "not symbolic" and fell through to update-ref -d. But update-ref -d on a symbolic ref deletes the ref it points at, not the symref itself (e.g. update-ref -d HEAD deletes the current branch). A transient probe failure (timeout, spawn error, I/O) on a symbolic ref would therefore be routed into a destructive delete of the wrong ref, silently losing a branch pointer. Apply the same refLookupAbsent classification used for lookups: a genuine "not a symbolic ref / not found" is exit-non-zero with empty stderr and falls through to update-ref -d; a spawn/timeout/I-O failure is surfaced instead of being assumed non-symbolic. The Reference and IterReferences symbolic-ref probes do not share this hazard: their failure falls through to a classified rev-parse that surfaces genuine failures, and the path is a non-destructive read. Mutation-verified test injects failing symbolic-ref probes and asserts the error is surfaced and never routed into update-ref -d.
…able
Two related robustness fixes to the reftable git-CLI ref storer:
1. Force LC_ALL=C / LANG=C for every reftable git plumbing invocation (new
gitPlumbingEnv). git's diagnostics are i18n'd, and the reftable error
classification matches English substrings ("is at X but expected Y",
"reference already exists", "does not exist"). On a localized machine
those messages are translated, so isRefCASConflict and RemoveReference
idempotency would silently misclassify CAS conflicts and delete
outcomes. Mirrors checkpoint/shadow_ref.go. refLookupAbsent already keys
on exit code + empty stderr and is locale-independent.
2. Classify the symbolic-ref -q probe failure in Reference and
IterReferences, matching the RemoveReference fix. Previously any probe
error fell through to the hash path, so a transient failure (timeout,
spawn, I/O) on a symbolic ref such as HEAD downgraded it to a Hash
reference literally named "HEAD" — making go-git's ResolveReference stop
there and callers that check head.Name().IsBranch() read the repo as
detached. Now a genuine "not symbolic" (exit non-zero, empty stderr)
falls through, but a real git failure is surfaced.
Mutation-verified tests: locale env forcing overrides inherited
LANG/LC_ALL, refLookupAbsent locale-independence, and transient
symbolic-probe surfacing for both Reference and IterReferences.
Detach runCLIIn's spawned entire binary from any controlling TTY via execx.NonInteractive, matching TestEnv.RunCLIWithError and every other integration helper, so the linked-worktree reftable test can't hang on an interactive prompt path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://entire.io/gh/entireio/cli/trails/830
Summary
entire enable(and every subsequent capture operation) aborts on any repository that uses the reftable ref backend:Reftable is created by
git init --ref-format=reftableandgit refs migrate --ref-format=reftable, and is slated to become git's default. This PR makes the full enable → session → commit → checkpoint flow work on reftable repositories. Fixes #547.Root cause
The vendored
go-git/go-git/v6alpha has no reftable reader:.git/refs,.git/packed-refsand.git/HEAD— in a reftable repo the real refs live in a binary reftable stack and.git/HEADis only aref: refs/heads/.invalidstub;verifyExtensions) rejectsextensions.refstorage=reftableoutright, which is the exact error users see.Upstream is not fixing this soon: go-git PR #1970 ("Reftable support") was closed unmerged, and go-git issue #1827 is still open. Waiting on the dependency is not viable.
Approach
Detect reftable repositories in
gitrepo.OpenPath(presence of areftable/directory under the worktree or common git dir) and wrap go-git's filesystem storage with areftableStorer:ReferenceStorermethods (SetReference,CheckAndSetReference,Reference,IterReferences,RemoveReference,CountLooseRefs,PackRefs) are overridden to shell out to git ref plumbing (update-ref,symbolic-ref,rev-parse,for-each-ref);SupportsExtension("refstorage")so go-git's extension check passes on open.This extends the codebase's existing "prefer the git CLI where go-git is unsafe or unsupported" seam (already used for
reset/checkoutviaHardResetWithProtection, and forfetch) rather than inventing a new architecture. Because every ref read/write in the CLI funnels throughgitrepo.OpenPath, one wrapper makes all callers — enable, session start, shadow branches, condensation, theentire/checkpoints/v1branch, andcheckpoint list— work unchanged. The ref plumbing used does not fire git hooks, so it cannot recurse back intoentire.Scope note: this covers the local capture flow. Network transport of reftable stacks is out of scope and unaffected (push/fetch already route through the git CLI). No behavior changes for classic files-backend or sha256 repositories.
Testing
Unit tests (
cmd/entire/cli/gitrepo/reftable_test.go) and integration tests (cmd/entire/cli/integration_test/reftable_repo_test.go) are gated on git reftable support and skip on older git (same pattern as the existing sha256 repo test). Live-verified against the built binary with real git state inspection:entire enableentire/checkpoints/v1Entire-Checkpoint:trailerentire checkpoint listGates:
gofmtclean,golangci-lintclean on touched packages, and the full integration + strategy suites pass (1119 tests).Hardening (audit follow-up)
A follow-up commit hardens the ref-plumbing seam after an adversarial review:
CheckAndSetReferencepreviously mapped everygit update-reffailure tostorage.ErrReferenceHasChanged.strategy.atomicSetV1Refkeys a privacy-critical pre-push decision on that sentinel (concurrency conflict → abort asV1RefMovedError) and documents that all other errors must be surfaced as-is. The coarse mapping misreported storage errors (nonexistent object, invalid ref name, lock contention, timeout, git spawn failure) as concurrency conflicts, producing a bogusV1RefMovedError("ref moved from X to X"). Now only a genuine compare-and-swap conflict (is at X but expected Y, orreference already existsfor a create race) maps to the sentinel; every other failure is wrapped and surfaced as itself.--end-of-options, so a name beginning with-is treated as a literal argument rather than a git option. Args were already passed as argv (never a shell string), so there was no command-injection exposure; this closes off option-injection for exotic ref names.Reference, theIterReferencesHEAD path, andRemoveReferencepreviously treated every git failure as a definitive negative — a spawn failure, thereftableGitTimeoutkill, or an I/O error was reported asErrReferenceNotFound/ a silently-dropped HEAD / a successful deletion. Becausestrategy.SafelyAdvanceLocalRefblind-overwrites a ref it believes absent (no CAS), a transient git failure on a critical ref (e.g.entire/checkpoints/v1) could discard real history. A newrefLookupAbsentclassifier maps only a genuine "git ran and exited non-zero with empty stderr" (git's--quietnot-found contract) to absence; timeouts (now re-wrapped with the context error), spawn/I-O errors, and non-zero exits carrying stderr are surfaced.RemoveReferenceno longer swallows an empty-stderr failure (git exits 0 for an already-absent ref).