From 8c8c7ea0ca03fa0ffefbdaddb01c5ded9156f4b4 Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Wed, 15 Jul 2026 18:20:29 -0400 Subject: [PATCH 1/6] feat(checkpoint): enumerate remote checkpoint refs in git-refs List Under a git-refs primary, a second device running `entire checkpoint list` saw zero refs-native checkpoints: storage-level List enumerated local refs only, and the on-demand ref fetcher is ID-keyed (it can fetch a known checkpoint, not discover which exist). PR #1719 healed this for the git-branch backend by fetching the v1 metadata branch, but refs- native checkpoints do not live on that branch, so the symptom survived. Add opt-in remote discovery to the git-refs List: when the context is marked with WithRemoteListDiscovery and a remote lister is configured, List runs a single `git ls-remote refs/entire/checkpoints/*` (names only, no object transfer) and surfaces checkpoints that exist on the remote but have no local ref yet. Each discovered checkpoint's CreatedAt is recovered from its ULID timestamp so it sorts correctly, and its contents are hydrated lazily on the next read via the existing ID-keyed fetch. Enumeration follows the same authority rule as #1719: with a checkpoint_remote configured it queries that remote, never origin; with none configured it is a no-op, so List stays local-only and default behavior is unchanged. The discovery marker is set only on user-facing enumeration (`checkpoint list` / the branch `explain` view), never the per-turn commit hook, so the hot path stays network-free. Enumeration is best-effort: an ls-remote failure logs and returns the local list. Closes #1770 Co-authored-by: Cursor --- cmd/entire/cli/checkpoint/fetching_tree.go | 11 ++ cmd/entire/cli/checkpoint/id/id.go | 19 +++ cmd/entire/cli/checkpoint/id/id_test.go | 38 ++++++ cmd/entire/cli/checkpoint/open.go | 9 +- cmd/entire/cli/checkpoint/refs_store.go | 96 ++++++++++++++- cmd/entire/cli/checkpoint/refs_store_test.go | 112 ++++++++++++++++++ cmd/entire/cli/checkpoint/registry.go | 17 ++- .../cli/checkpoint/routing_store_test.go | 31 +++++ cmd/entire/cli/explain.go | 14 ++- cmd/entire/cli/git_operations.go | 57 +++++++++ cmd/entire/cli/git_operations_test.go | 81 +++++++++++++ docs/architecture/ref-checkpoint-backend.md | 14 ++- 12 files changed, 484 insertions(+), 15 deletions(-) diff --git a/cmd/entire/cli/checkpoint/fetching_tree.go b/cmd/entire/cli/checkpoint/fetching_tree.go index 891ae16586..d83dd4171d 100644 --- a/cmd/entire/cli/checkpoint/fetching_tree.go +++ b/cmd/entire/cli/checkpoint/fetching_tree.go @@ -23,6 +23,17 @@ type BlobFetchFunc func(ctx context.Context, hashes []plumbing.Hash) error // package cannot resolve the remote target itself, so the CLI layer injects it. type RefFetchFunc func(ctx context.Context, ref plumbing.ReferenceName) error +// RemoteRefListFunc enumerates the per-checkpoint refs present on the configured +// checkpoint remote (names only, via `ls-remote refs/entire/checkpoints/*` — no +// object transfer), returning their full ref names. The git-refs store uses it +// in List to discover checkpoints written on another machine that have no local +// ref yet; each discovered checkpoint is then hydrated lazily on read via +// RefFetchFunc. The checkpoint package cannot resolve the remote target itself, +// so the CLI layer injects it. Enumeration is checkpoint-remote-scoped (never +// origin): it returns (nil, nil) when no checkpoint remote is configured, which +// leaves List local-only — the same authority rule as the on-demand fetch. +type RemoteRefListFunc func(ctx context.Context) ([]plumbing.ReferenceName, error) + // FetchingTree wraps a git tree to automatically fetch missing blobs on demand. // After a treeless fetch (--filter=blob:none), tree objects are available locally // but blob objects are not. Each File() call checks whether the target blob diff --git a/cmd/entire/cli/checkpoint/id/id.go b/cmd/entire/cli/checkpoint/id/id.go index adbb6239a8..7e23736f17 100644 --- a/cmd/entire/cli/checkpoint/id/id.go +++ b/cmd/entire/cli/checkpoint/id/id.go @@ -8,6 +8,7 @@ import ( "encoding/json" "fmt" "regexp" + "time" ulid "github.com/oklog/ulid/v2" ) @@ -224,6 +225,24 @@ func (id CheckpointID) IsEmpty() bool { return id == EmptyCheckpointID } +// Time returns the creation time encoded in this ID and whether one is +// available. A ULID embeds a millisecond Unix timestamp in its leading +// characters, so the time is recoverable from the ID alone — no store read +// required. This is what lets remote-ref discovery (which learns only ref names +// via ls-remote) present and sort a not-yet-hydrated checkpoint by its real +// creation time. Legacy 12-hex IDs carry no timestamp, so this returns +// (zero, false) for them. +func (id CheckpointID) Time() (time.Time, bool) { + if id.Kind() != KindULID { + return time.Time{}, false + } + u, err := ulid.ParseStrict(string(id)) + if err != nil { + return time.Time{}, false + } + return ulid.Time(u.Time()), true +} + // Path returns the sharded path for this checkpoint ID on entire/checkpoints/v1. // Uses first 2 characters as shard (256 buckets), remaining as folder name. // Example: "a3b2c4d5e6f7" -> "a3/b2c4d5e6f7" diff --git a/cmd/entire/cli/checkpoint/id/id_test.go b/cmd/entire/cli/checkpoint/id/id_test.go index f96e601d60..c55a14db34 100644 --- a/cmd/entire/cli/checkpoint/id/id_test.go +++ b/cmd/entire/cli/checkpoint/id/id_test.go @@ -3,11 +3,49 @@ package id import ( "encoding/json" "testing" + "time" + + ulid "github.com/oklog/ulid/v2" ) // A representative ULID (Crockford base32, 26 chars) used across tests. const sampleULID = "01KVBJCWYA4YW6J5M9GP655HZN" +func TestCheckpointID_Time(t *testing.T) { + t.Parallel() + + // A ULID minted from a known instant recovers that instant (millisecond + // precision), so remote-ref discovery can sort/display a checkpoint by its + // real creation time from the ref name alone — no store read. + want := time.UnixMilli(1700000000000).UTC() + minted := ulid.MustNew(ulid.Timestamp(want), nil) + got, ok := CheckpointID(minted.String()).Time() + if !ok { + t.Fatalf("Time() ok = false for a valid ULID %q", minted) + } + if !got.Equal(want) { + t.Errorf("Time() = %v, want %v", got, want) + } + + // The canonical sample ULID also yields a non-zero time. + if ts, ok := CheckpointID(sampleULID).Time(); !ok || ts.IsZero() { + t.Errorf("Time() for sample ULID = (%v, %v), want a non-zero time", ts, ok) + } + + // A legacy hex ID carries no timestamp. + if _, ok := CheckpointID("a1b2c3d4e5f6").Time(); ok { + t.Errorf("Time() ok = true for a legacy hex ID; want false") + } + + // Non-ID / empty strings report no time. + if _, ok := CheckpointID("").Time(); ok { + t.Errorf("Time() ok = true for empty ID; want false") + } + if _, ok := CheckpointID("not-an-id").Time(); ok { + t.Errorf("Time() ok = true for a non-ID string; want false") + } +} + func TestGenerateULID(t *testing.T) { t.Parallel() a, err := GenerateULID() diff --git a/cmd/entire/cli/checkpoint/open.go b/cmd/entire/cli/checkpoint/open.go index 13e954b922..640b7b2ac8 100644 --- a/cmd/entire/cli/checkpoint/open.go +++ b/cmd/entire/cli/checkpoint/open.go @@ -24,6 +24,13 @@ type OpenOptions struct { // reads local-only; ignored by the git-branch backend. RefFetcher RefFetchFunc + // RemoteRefLister is the CLI-level checkpoint-ref enumerator, used by the + // git-refs backend's List to discover checkpoints present on the checkpoint + // remote but not yet local (see RemoteRefListFunc). It only fires on a + // context marked by WithRemoteListDiscovery. nil (or an unmarked context) + // leaves List local-only; ignored by the git-branch backend. + RemoteRefLister RemoteRefListFunc + // Refs overrides the default committed-ref topology. A non-nil value wins, // e.g. attach pins reads to Primary via PrimaryAsRead(). Refs *PersistentRefs @@ -62,7 +69,7 @@ type Stores struct { // default git-branch backend with no mirrors, preserving default behavior. func Open(ctx context.Context, repo *git.Repository, opts OpenOptions) (*Stores, error) { refs := resolveOpenRefs(ctx, opts) - env := OpenEnv{Repo: repo, BlobFetcher: opts.BlobFetcher, RefFetcher: opts.RefFetcher, Refs: refs} + env := OpenEnv{Repo: repo, BlobFetcher: opts.BlobFetcher, RefFetcher: opts.RefFetcher, RemoteRefLister: opts.RemoteRefLister, Refs: refs} cfg, err := settings.LoadCheckpointsConfig(ctx) if err != nil { diff --git a/cmd/entire/cli/checkpoint/refs_store.go b/cmd/entire/cli/checkpoint/refs_store.go index a8849b965d..9e5a471823 100644 --- a/cmd/entire/cli/checkpoint/refs_store.go +++ b/cmd/entire/cli/checkpoint/refs_store.go @@ -32,8 +32,9 @@ var ( type gitRefsStore struct { *treeWriter - blobFetcher BlobFetchFunc - refFetcher RefFetchFunc + blobFetcher BlobFetchFunc + refFetcher RefFetchFunc + remoteRefLister RemoteRefListFunc } // newGitRefsStore constructs the per-checkpoint-ref store for a repository. @@ -52,6 +53,38 @@ func (s *gitRefsStore) SetRefFetcher(f RefFetchFunc) { s.refFetcher = f } +// SetRemoteRefLister configures remote checkpoint-ref enumeration for List (see +// RemoteRefListFunc). It only takes effect when List is called on a context +// marked by WithRemoteListDiscovery, so the per-turn hook hot path — which +// lists local refs without opting in — never triggers a network round trip. nil +// leaves List local-only. +func (s *gitRefsStore) SetRemoteRefLister(f RemoteRefListFunc) { + s.remoteRefLister = f +} + +// remoteListDiscoveryKey marks a context as permitting List to enumerate the +// checkpoint remote. It is an unexported key type so only this package can set +// or read the marker. +type remoteListDiscoveryKey struct{} + +// WithRemoteListDiscovery marks ctx to allow gitRefsStore.List to enumerate +// checkpoint refs on the configured checkpoint remote (see RemoteRefListFunc) +// and surface not-yet-local checkpoints. Set it only on explicit, user-facing +// enumeration flows (e.g. `entire checkpoint list` / the branch `explain` +// view), never on the per-turn commit hook: routine local listings must stay +// network-free. Without this marker List is local-only regardless of whether a +// remote lister is configured. +func WithRemoteListDiscovery(ctx context.Context) context.Context { + return context.WithValue(ctx, remoteListDiscoveryKey{}, true) +} + +// remoteListDiscoveryEnabled reports whether ctx was marked via +// WithRemoteListDiscovery. +func remoteListDiscoveryEnabled(ctx context.Context) bool { + v, ok := ctx.Value(remoteListDiscoveryKey{}).(bool) + return ok && v +} + // Write dispatches a persistent write request to the matching ref operation, // mirroring the git-branch store's Write. func (s *gitRefsStore) Write(ctx context.Context, req WriteRequest) error { @@ -357,8 +390,17 @@ func (s *gitRefsStore) ReadSessionContent(ctx context.Context, checkpointID id.C } // List enumerates local checkpoint refs and reads each root summary, sorted most -// recent first. Storage-level listing is local-refs-only for now (no remote -// enumeration), matching the issue's first-version scope. +// recent first. +// +// When the context opts in (WithRemoteListDiscovery) and a remote ref lister is +// configured, it additionally discovers checkpoints that exist on the +// checkpoint remote but have no local ref yet — the "second device sees zero +// checkpoints" case. Discovery is names-only (an ls-remote of +// refs/entire/checkpoints/*, no object transfer): each remote-only checkpoint is +// listed from its ref name alone and hydrated lazily on a later read via the +// on-demand ref fetch. Remote enumeration is best-effort and additive — a +// failure logs and leaves the local results intact rather than failing the +// whole listing. func (s *gitRefsStore) List(ctx context.Context) ([]CheckpointInfo, error) { if err := ctx.Err(); err != nil { return nil, err //nolint:wrapcheck // Propagating context cancellation @@ -371,6 +413,7 @@ func (s *gitRefsStore) List(ctx context.Context) ([]CheckpointInfo, error) { defer refs.Close() var checkpoints []CheckpointInfo + seen := make(map[id.CheckpointID]struct{}) err = refs.ForEach(func(ref *plumbing.Reference) error { cid, ok := ParseRef(ref.Name()) if !ok { @@ -385,16 +428,61 @@ func (s *gitRefsStore) List(ctx context.Context) ([]CheckpointInfo, error) { return nil //nolint:nilerr // skip unreadable refs, keep listing } checkpoints = append(checkpoints, readCommittedInfoFromCheckpointTree(cid, tree)) + seen[cid] = struct{}{} return nil }) if err != nil { return nil, fmt.Errorf("iterate checkpoint refs: %w", err) } + if s.remoteRefLister != nil && remoteListDiscoveryEnabled(ctx) { + checkpoints = s.appendRemoteDiscovered(ctx, checkpoints, seen) + } + sortCheckpointInfosByRecency(checkpoints) return checkpoints, nil } +// appendRemoteDiscovered enumerates checkpoint refs on the configured checkpoint +// remote and appends any that are not present locally (tracked in seen) as +// not-yet-hydrated CheckpointInfos. It never fetches objects: the ref name +// yields the checkpoint ID, and a ULID ID yields its creation time, so a +// discovered checkpoint sorts and displays correctly before its first read +// hydrates the rest. Best-effort: an enumeration failure logs and returns the +// unchanged local list. +func (s *gitRefsStore) appendRemoteDiscovered(ctx context.Context, checkpoints []CheckpointInfo, seen map[id.CheckpointID]struct{}) []CheckpointInfo { + remoteRefs, err := s.remoteRefLister(ctx) + if err != nil { + logging.Warn(ctx, "git-refs: remote checkpoint enumeration failed; listing local refs only", + slog.String("error", err.Error())) + return checkpoints + } + for _, refName := range remoteRefs { + cid, ok := ParseRef(refName) + if !ok { + continue + } + if _, dup := seen[cid]; dup { + continue + } + seen[cid] = struct{}{} + checkpoints = append(checkpoints, remoteDiscoveredInfo(cid)) + } + return checkpoints +} + +// remoteDiscoveredInfo builds the minimal CheckpointInfo for a checkpoint known +// only by its remote ref name. Its contents are not fetched here (that happens +// lazily on read); CreatedAt is recovered from the ULID timestamp so the entry +// sorts by real creation time, and is left zero for a (rare) legacy-hex ref. +func remoteDiscoveredInfo(cid id.CheckpointID) CheckpointInfo { + info := CheckpointInfo{CheckpointID: cid} + if createdAt, ok := cid.Time(); ok { + info.CreatedAt = createdAt + } + return info +} + // GetCheckpointAuthor returns the author of the checkpoint ref's tip commit (the // most recent writer). Returns a zero Author when the ref is absent. func (s *gitRefsStore) GetCheckpointAuthor(ctx context.Context, checkpointID id.CheckpointID) (Author, error) { diff --git a/cmd/entire/cli/checkpoint/refs_store_test.go b/cmd/entire/cli/checkpoint/refs_store_test.go index 3cd101504c..ded9021fbb 100644 --- a/cmd/entire/cli/checkpoint/refs_store_test.go +++ b/cmd/entire/cli/checkpoint/refs_store_test.go @@ -118,6 +118,118 @@ func TestGitRefsStore_OnDemandRefFetch_FailurePropagates(t *testing.T) { }) } +// TestGitRefsStore_ListRemoteDiscovery exercises the git-refs List remote-ref +// discovery that fixes #1770: on a second device, a checkpoint written +// elsewhere has no local ref, so a purely local List shows zero. With discovery +// opted in (WithRemoteListDiscovery) and a remote lister configured, List +// enumerates the checkpoint remote (names only) and surfaces the not-yet-local +// checkpoint; a later read hydrates it. +func TestGitRefsStore_ListRemoteDiscovery(t *testing.T) { + t.Parallel() + + // A ULID that exists only "on the remote" (never written locally). + remoteOnly := id.CheckpointID("01KVBJCWYA4YW6J5M9GP655HZN") + remoteOnlyRef := mustRefName(t, remoteOnly) + //nolint:unparam // test fake mirrors RemoteRefListFunc's (…, error) signature; it always succeeds here. + lister := func(context.Context) ([]plumbing.ReferenceName, error) { + return []plumbing.ReferenceName{remoteOnlyRef}, nil + } + + ids := func(infos []CheckpointInfo) map[id.CheckpointID]struct{} { + out := make(map[id.CheckpointID]struct{}, len(infos)) + for _, info := range infos { + out[info.CheckpointID] = struct{}{} + } + return out + } + + t.Run("discovers remote-only checkpoint when opted in", func(t *testing.T) { + t.Parallel() + store := newRefsStore(t) + local := id.MustCheckpointID("a1b2c3d4e5f6") + refsWrite(t, store, local, "s-local", "t") + store.SetRemoteRefLister(lister) + + infos, err := store.List(WithRemoteListDiscovery(context.Background())) + require.NoError(t, err) + got := ids(infos) + assert.Contains(t, got, local, "local checkpoint still listed") + assert.Contains(t, got, remoteOnly, "remote-only checkpoint discovered via ls-remote") + + // The discovered entry carries the ULID's embedded creation time, so it + // sorts by real recency without an object fetch. + for _, info := range infos { + if info.CheckpointID == remoteOnly { + assert.False(t, info.CreatedAt.IsZero(), "discovered ULID checkpoint should carry its embedded creation time") + } + } + }) + + t.Run("stays local-only without the discovery marker", func(t *testing.T) { + t.Parallel() + store := newRefsStore(t) + local := id.MustCheckpointID("a1b2c3d4e5f6") + refsWrite(t, store, local, "s-local", "t") + store.SetRemoteRefLister(lister) + + infos, err := store.List(context.Background()) + require.NoError(t, err) + got := ids(infos) + assert.Contains(t, got, local) + assert.NotContains(t, got, remoteOnly, "no enumeration without WithRemoteListDiscovery (keeps the hot path network-free)") + }) + + t.Run("stays local-only when no lister is configured", func(t *testing.T) { + t.Parallel() + store := newRefsStore(t) + local := id.MustCheckpointID("a1b2c3d4e5f6") + refsWrite(t, store, local, "s-local", "t") + + infos, err := store.List(WithRemoteListDiscovery(context.Background())) + require.NoError(t, err) + got := ids(infos) + assert.Contains(t, got, local) + assert.NotContains(t, got, remoteOnly) + }) + + t.Run("does not duplicate a checkpoint already present locally", func(t *testing.T) { + t.Parallel() + store := newRefsStore(t) + local := id.MustCheckpointID("a1b2c3d4e5f6") + refsWrite(t, store, local, "s-local", "t") + // The lister also advertises the checkpoint that already exists locally. + store.SetRemoteRefLister(func(context.Context) ([]plumbing.ReferenceName, error) { + return []plumbing.ReferenceName{mustRefName(t, local), remoteOnlyRef}, nil + }) + + infos, err := store.List(WithRemoteListDiscovery(context.Background())) + require.NoError(t, err) + count := 0 + for _, info := range infos { + if info.CheckpointID == local { + count++ + } + } + assert.Equal(t, 1, count, "a locally-present checkpoint advertised by the remote is not duplicated") + }) + + t.Run("enumeration failure degrades to local-only", func(t *testing.T) { + t.Parallel() + store := newRefsStore(t) + local := id.MustCheckpointID("a1b2c3d4e5f6") + refsWrite(t, store, local, "s-local", "t") + store.SetRemoteRefLister(func(context.Context) ([]plumbing.ReferenceName, error) { + return nil, assert.AnError // e.g. offline / ls-remote failed + }) + + infos, err := store.List(WithRemoteListDiscovery(context.Background())) + require.NoError(t, err, "a remote enumeration failure must not fail the whole listing") + got := ids(infos) + assert.Contains(t, got, local, "local checkpoints remain listed when discovery fails") + assert.NotContains(t, got, remoteOnly) + }) +} + func TestGitRefsStore_WriteAllVariantsAndRead(t *testing.T) { t.Parallel() store := newRefsStore(t) diff --git a/cmd/entire/cli/checkpoint/registry.go b/cmd/entire/cli/checkpoint/registry.go index 20174bb06f..ca3119927d 100644 --- a/cmd/entire/cli/checkpoint/registry.go +++ b/cmd/entire/cli/checkpoint/registry.go @@ -27,13 +27,15 @@ const BackendTypeGitBranch = "git-branch" const BackendTypeGitRefs = "git-refs" // OpenEnv carries the construction context a backend factory may need. -// Git-backed backends require Repo and use Refs/BlobFetcher/RefFetcher; other -// backends ignore the git-shaped fields and read their own configuration from cfg. +// Git-backed backends require Repo and use Refs/BlobFetcher/RefFetcher/ +// RemoteRefLister; other backends ignore the git-shaped fields and read their +// own configuration from cfg. type OpenEnv struct { - Repo *git.Repository - BlobFetcher BlobFetchFunc - RefFetcher RefFetchFunc - Refs PersistentRefs + Repo *git.Repository + BlobFetcher BlobFetchFunc + RefFetcher RefFetchFunc + RemoteRefLister RemoteRefListFunc + Refs PersistentRefs } // Factory constructs a persistent store for one backend type. cfg is the @@ -163,5 +165,8 @@ func gitRefsBackendFactory(_ context.Context, env OpenEnv, _ json.RawMessage) (P if env.RefFetcher != nil { store.SetRefFetcher(env.RefFetcher) } + if env.RemoteRefLister != nil { + store.SetRemoteRefLister(env.RemoteRefLister) + } return store, nil } diff --git a/cmd/entire/cli/checkpoint/routing_store_test.go b/cmd/entire/cli/checkpoint/routing_store_test.go index 21760ab8f5..886ba6101d 100644 --- a/cmd/entire/cli/checkpoint/routing_store_test.go +++ b/cmd/entire/cli/checkpoint/routing_store_test.go @@ -194,6 +194,37 @@ func TestKindRoutingStore_ListDedupesAcrossBackends(t *testing.T) { assert.Equal(t, 1, count, "a checkpoint present in both backends should appear once") } +// TestKindRoutingStore_ListSurfacesRemoteDiscovery proves the git-refs remote +// discovery flows through the routing store: the routing List delegates to the +// refs store with the caller's context, so a WithRemoteListDiscovery context +// makes a checkpoint present only on the remote appear in the unioned list. +func TestKindRoutingStore_ListSurfacesRemoteDiscovery(t *testing.T) { + t.Parallel() + _, repo, _ := newTestRepo(t) + branch := NewGitStore(repo, DefaultV1Refs()) + refs := newGitRefsStore(repo) + + localULID := id.MustCheckpointID(routingSampleULID) + writeRoutingCheckpoint(t, refs, localULID, "ulid-in-refs") + + // A different ULID that lives only on the remote (no local ref). + remoteOnly := id.MustCheckpointID("01KVBJCWYA4YW6J5M9GP655HYY") + refs.SetRemoteRefLister(func(context.Context) ([]plumbing.ReferenceName, error) { + return []plumbing.ReferenceName{mustRefName(t, remoteOnly)}, nil + }) + + router := newKindRoutingStore(refs, branch, refs, BackendTypeGitRefs) + + infos, err := router.List(WithRemoteListDiscovery(context.Background())) + require.NoError(t, err) + seen := make(map[id.CheckpointID]struct{}, len(infos)) + for _, info := range infos { + seen[info.CheckpointID] = struct{}{} + } + assert.Contains(t, seen, localULID, "local refs checkpoint is listed") + assert.Contains(t, seen, remoteOnly, "remote-only checkpoint is discovered through the routing store") +} + func TestKindRoutingStore_GetCheckpointAuthorRoutes(t *testing.T) { t.Parallel() ctx := context.Background() diff --git a/cmd/entire/cli/explain.go b/cmd/entire/cli/explain.go index fc286557b1..5bc8b65d3c 100644 --- a/cmd/entire/cli/explain.go +++ b/cmd/entire/cli/explain.go @@ -2044,14 +2044,24 @@ func getBranchCheckpoints(ctx context.Context, repo *git.Repository, limit int) // Warn (once per process) if metadata branches are disconnected strategy.WarnIfMetadataDisconnected() - stores, err := checkpoint.Open(ctx, repo, checkpoint.OpenOptions{}) + // This is a user-facing enumeration (`entire checkpoint list` / the branch + // `explain` view), so opt into git-refs remote discovery: when a + // checkpoint_remote is configured, List enumerates it (names only) to + // surface refs-native checkpoints written on another machine, and the + // fetchers hydrate each on read. WithRemoteListDiscovery keeps this off the + // per-turn hook hot path. + stores, err := checkpoint.Open(ctx, repo, checkpoint.OpenOptions{ + BlobFetcher: FetchBlobsByHash, + RefFetcher: FetchCheckpointRef, + RemoteRefLister: ListCheckpointRefsOnRemote, + }) if err != nil { return nil, false, fmt.Errorf("open checkpoint store: %w", err) } store := stores.Persistent // Get all committed checkpoints for lookup. - committedInfos, err := store.List(ctx) + committedInfos, err := store.List(checkpoint.WithRemoteListDiscovery(ctx)) if err != nil { committedInfos = nil // Continue without committed checkpoints } diff --git a/cmd/entire/cli/git_operations.go b/cmd/entire/cli/git_operations.go index a6ab8f1109..efdd048982 100644 --- a/cmd/entire/cli/git_operations.go +++ b/cmd/entire/cli/git_operations.go @@ -493,6 +493,63 @@ func FetchCheckpointRef(ctx context.Context, ref plumbing.ReferenceName) error { return nil } +// checkpointRefListTimeout bounds the ls-remote enumeration of checkpoint refs +// so a slow or unreachable checkpoint remote cannot hang `entire checkpoint +// list`; on timeout the store falls back to local refs only. +const checkpointRefListTimeout = 30 * time.Second + +// ListCheckpointRefsOnRemote enumerates the per-checkpoint refs +// (refs/entire/checkpoints//) present on the checkpoint remote, names +// only, via a single `git ls-remote refs/entire/checkpoints/*` — no object +// transfer. The git-refs store's List uses it to discover checkpoints written +// on another machine that have no local ref yet, then hydrates each lazily on +// read through FetchCheckpointRef. +// +// Enumeration is checkpoint-remote-scoped, matching the on-demand fetch and PR +// #1719's authority rule: with a checkpoint_remote configured it queries that +// remote, and with none configured it returns (nil, nil) so List stays +// local-only rather than scanning origin. This keeps the default (no +// checkpoint_remote) behavior unchanged. +func ListCheckpointRefsOnRemote(ctx context.Context) ([]plumbing.ReferenceName, error) { + if !remote.Configured(ctx) { + return nil, nil + } + + url, err := remote.FetchURL(ctx) + if err != nil { + return nil, fmt.Errorf("resolve checkpoint remote URL: %w", err) + } + + ctx, cancel := context.WithTimeout(ctx, checkpointRefListTimeout) + defer cancel() + + output, err := remote.LsRemoteInDir(ctx, "", url, checkpoint.CheckpointRefPrefix+"*") + if err != nil { + return nil, fmt.Errorf("ls-remote checkpoint refs from %s: %w", remote.RedactURL(url), err) + } + return parseCheckpointRefNames(output), nil +} + +// parseCheckpointRefNames extracts the checkpoint ref names from `git ls-remote` +// output. Each line is "\t"; only refs under CheckpointRefPrefix +// are kept (the store re-validates each via ParseRef). Peeled tag lines +// ("\t^{}") never carry the checkpoint prefix, so they drop out. +func parseCheckpointRefNames(output []byte) []plumbing.ReferenceName { + var names []plumbing.ReferenceName + for _, line := range strings.Split(string(output), "\n") { + fields := strings.Fields(line) + if len(fields) < 2 { + continue + } + name := fields[1] + if !strings.HasPrefix(name, checkpoint.CheckpointRefPrefix) { + continue + } + names = append(names, plumbing.ReferenceName(name)) + } + return names +} + // FetchBlobsByHash fetches specific blob objects from the remote by their SHA-1 hashes. // Uses "git fetch " which goes through normal credential helpers, // unlike fetch-pack which bypasses them. Requires the server to support diff --git a/cmd/entire/cli/git_operations_test.go b/cmd/entire/cli/git_operations_test.go index 588740174c..2f6c502f6e 100644 --- a/cmd/entire/cli/git_operations_test.go +++ b/cmd/entire/cli/git_operations_test.go @@ -8,6 +8,8 @@ import ( "strings" "testing" + "github.com/entireio/cli/cmd/entire/cli/checkpoint" + "github.com/entireio/cli/cmd/entire/cli/checkpoint/remote" "github.com/entireio/cli/cmd/entire/cli/testutil" "github.com/go-git/go-git/v6" @@ -635,3 +637,82 @@ func gitDefaultBranch(t *testing.T, dir string) string { t.Helper() return gitOutput(t, dir, "rev-parse", "--abbrev-ref", "HEAD") } + +// TestParseCheckpointRefNames verifies the ls-remote parser keeps only +// checkpoint refs and ignores unrelated advertisement lines (HEAD, branches, +// peeled tags) and blanks. +func TestParseCheckpointRefNames(t *testing.T) { + t.Parallel() + const sha = "e9ed0bd3ad3b2071aefab6e6ad20527dc910957b" + output := []byte(strings.Join([]string{ + sha + "\tHEAD", + sha + "\trefs/heads/main", + sha + "\trefs/entire/checkpoints/ZN/01KVBJCWYA4YW6J5M9GP655HZN", + sha + "\trefs/entire/checkpoints/f6/a1b2c3d4e5f6", + sha + "\trefs/tags/v1.0.0", + sha + "\trefs/tags/v1.0.0^{}", + "", + }, "\n")) + + names := parseCheckpointRefNames(output) + got := make([]string, len(names)) + for i, n := range names { + got[i] = n.String() + } + assert.ElementsMatch(t, []string{ + "refs/entire/checkpoints/ZN/01KVBJCWYA4YW6J5M9GP655HZN", + "refs/entire/checkpoints/f6/a1b2c3d4e5f6", + }, got) +} + +// TestParseCheckpointRefNames_RealLsRemote exercises the parser against genuine +// `git ls-remote 'refs/entire/checkpoints/*'` output from a local bare remote, +// confirming the glob matches the nested / refs and nothing else. +func TestParseCheckpointRefNames_RealLsRemote(t *testing.T) { + t.Parallel() + ctx := context.Background() + + bareDir := t.TempDir() + gitRun(t, bareDir, "init", "--bare", "-q", bareDir) + + workDir := t.TempDir() + testutil.InitRepo(t, workDir) + testutil.WriteFile(t, workDir, "f.txt", "init") + testutil.GitAdd(t, workDir, "f.txt") + testutil.GitCommit(t, workDir, "init") + head := gitOutput(t, workDir, "rev-parse", "HEAD") + gitRun(t, workDir, "update-ref", "refs/entire/checkpoints/ZN/01KVBJCWYA4YW6J5M9GP655HZN", head) + gitRun(t, workDir, "update-ref", "refs/entire/checkpoints/f6/a1b2c3d4e5f6", head) + gitRun(t, workDir, "remote", "add", "origin", bareDir) + gitRun(t, workDir, "push", "-q", "origin", "refs/entire/checkpoints/*:refs/entire/checkpoints/*") + + out, err := remote.LsRemoteInDir(ctx, workDir, bareDir, checkpoint.CheckpointRefPrefix+"*") + require.NoError(t, err) + + names := parseCheckpointRefNames(out) + got := make([]string, len(names)) + for i, n := range names { + got[i] = n.String() + } + assert.ElementsMatch(t, []string{ + "refs/entire/checkpoints/ZN/01KVBJCWYA4YW6J5M9GP655HZN", + "refs/entire/checkpoints/f6/a1b2c3d4e5f6", + }, got) +} + +// TestListCheckpointRefsOnRemote_NotConfigured proves the authority gate: with +// no checkpoint_remote configured, enumeration is a no-op (nil, no error, no +// network) so List behavior stays unchanged (local-only) — never scanning +// origin. Not parallel: uses t.Chdir. +func TestListCheckpointRefsOnRemote_NotConfigured(t *testing.T) { + dir := t.TempDir() + testutil.InitRepo(t, dir) + testutil.WriteFile(t, dir, "f.txt", "init") + testutil.GitAdd(t, dir, "f.txt") + testutil.GitCommit(t, dir, "init") + t.Chdir(dir) + + names, err := ListCheckpointRefsOnRemote(context.Background()) + require.NoError(t, err) + assert.Nil(t, names, "no checkpoint_remote configured must leave List local-only (no remote enumeration)") +} diff --git a/docs/architecture/ref-checkpoint-backend.md b/docs/architecture/ref-checkpoint-backend.md index 65ca434e53..8d5bc0382e 100644 --- a/docs/architecture/ref-checkpoint-backend.md +++ b/docs/architecture/ref-checkpoint-backend.md @@ -118,7 +118,17 @@ A checkpoint written on another machine has no local ref. When a read misses loc - **genuinely absent** (remote has no such checkpoint) → maps to `ErrCheckpointNotFound`; - **a real failure** (IO, network, context cancellation) → returned as-is, never swallowed as "not found". -`List` at the storage level is **local-refs-only** — it enumerates local refs and reads each root summary. There is no remote enumeration. +### List and remote discovery + +`List` at the storage level enumerates **local refs** and reads each root summary. That alone is not enough on a **second device**: a checkpoint written elsewhere has no local ref, so a purely local `List` shows zero even though the checkpoint exists on the remote. Reads already fetch a *known* ID on demand (above), but `List` is a *discovery* problem — it has to learn which checkpoints exist before anything can read them. + +To close that gap `List` supports **opt-in remote discovery**: + +- The caller marks the context with `WithRemoteListDiscovery` (set only on explicit, user-facing enumeration — `entire checkpoint list` and the branch `explain` view — never the per-turn commit hook, so routine local listings stay network-free). +- A **remote ref lister** injected by the CLI runs a single `git ls-remote refs/entire/checkpoints/*` — **names only, no object transfer** — against the checkpoint remote. +- Each advertised ref that has no local ref yet is added to the result as a not-yet-hydrated `CheckpointInfo`. Its `CreatedAt` is recovered from the ULID timestamp in the ref name, so it sorts by real recency without a fetch; the rest of its contents are **hydrated lazily on the next read** via the on-demand ref fetch. + +Enumeration is **checkpoint-remote-scoped** — the same authority rule as the on-demand fetch and #1719: with a `checkpoint_remote` configured it queries that remote, and with **none** configured it does nothing (returns no refs), leaving `List` local-only rather than scanning origin. Discovery is also **best-effort and additive**: an `ls-remote` failure logs and returns the local results rather than failing the whole listing. ## Read routing and coexistence @@ -211,6 +221,6 @@ When checkpoints *are* actively migrated from the branch into refs (a path that ## Known limitations and deferred work -- **Storage-level `List` is local-only** — no remote enumeration of checkpoint refs. `List` at the routing layer still unions the two local stores. +- **Storage-level `List` is local-only by default**, with **opt-in remote discovery** for user-facing enumeration (see [List and remote discovery](#list-and-remote-discovery)): an `ls-remote` of `refs/entire/checkpoints/*` surfaces checkpoints written on another machine, hydrated lazily on read. It is scoped to a configured `checkpoint_remote` and kept off the per-turn hook hot path. - **OPF (OpenAI Privacy Filter) at pre-push is git-branch-only for now.** The per-ref push does not run OPF re-redaction; that is deferred until after the store lands. See `strategy/manual_commit_opf_rewrite.go` and [security-and-privacy.md](../security-and-privacy.md). - **The "ULIDs never land on the branch" invariant is not yet enforced at write time.** A config flip or a missing `ENTIRE_CHECKPOINTS_PRIMARY` in an amending environment could, in principle, condense a ULID checkpoint onto the `v1` branch, which readers (routing ULIDs to refs only) would then fail to find. Because git-branch is *not* a mirror of git-refs (see [Migration and coexistence](#migration-and-coexistence)), a ULID reaching the git-branch write path is unambiguously a bug — so enforcing this is a straightforward reject at that write path, not a topology-role-aware check. From db3974e315334136d63323504fa61c0c64c521ed Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Wed, 15 Jul 2026 18:55:14 -0400 Subject: [PATCH 2/6] fix(checkpoint): tighten remote list discovery for trail/Copilot Shorten ls-remote discovery timeout to 5s, pin FetchURL/ls-remote to the worktree root, and mint the ULID Time() fixture with deterministic entropy. Co-authored-by: Cursor --- cmd/entire/cli/checkpoint/id/id_test.go | 4 ++- cmd/entire/cli/git_operations.go | 23 +++++++++++----- cmd/entire/cli/git_operations_test.go | 30 +++++++++++++++++++++ docs/architecture/ref-checkpoint-backend.md | 2 +- 4 files changed, 51 insertions(+), 8 deletions(-) diff --git a/cmd/entire/cli/checkpoint/id/id_test.go b/cmd/entire/cli/checkpoint/id/id_test.go index c55a14db34..76632c1153 100644 --- a/cmd/entire/cli/checkpoint/id/id_test.go +++ b/cmd/entire/cli/checkpoint/id/id_test.go @@ -1,6 +1,7 @@ package id import ( + "bytes" "encoding/json" "testing" "time" @@ -18,7 +19,8 @@ func TestCheckpointID_Time(t *testing.T) { // precision), so remote-ref discovery can sort/display a checkpoint by its // real creation time from the ref name alone — no store read. want := time.UnixMilli(1700000000000).UTC() - minted := ulid.MustNew(ulid.Timestamp(want), nil) + // Deterministic entropy (zeros); Time() only reads the timestamp prefix. + minted := ulid.MustNew(ulid.Timestamp(want), bytes.NewReader(make([]byte, 16))) got, ok := CheckpointID(minted.String()).Time() if !ok { t.Fatalf("Time() ok = false for a valid ULID %q", minted) diff --git a/cmd/entire/cli/git_operations.go b/cmd/entire/cli/git_operations.go index efdd048982..1947c2459c 100644 --- a/cmd/entire/cli/git_operations.go +++ b/cmd/entire/cli/git_operations.go @@ -13,6 +13,7 @@ import ( "github.com/entireio/cli/cmd/entire/cli/checkpoint" "github.com/entireio/cli/cmd/entire/cli/checkpoint/remote" "github.com/entireio/cli/cmd/entire/cli/logging" + "github.com/entireio/cli/cmd/entire/cli/paths" "github.com/entireio/cli/cmd/entire/cli/strategy" "github.com/go-git/go-git/v6" @@ -493,10 +494,12 @@ func FetchCheckpointRef(ctx context.Context, ref plumbing.ReferenceName) error { return nil } -// checkpointRefListTimeout bounds the ls-remote enumeration of checkpoint refs -// so a slow or unreachable checkpoint remote cannot hang `entire checkpoint -// list`; on timeout the store falls back to local refs only. -const checkpointRefListTimeout = 30 * time.Second +// checkpointRefListTimeout bounds the names-only ls-remote used by user-facing +// `entire checkpoint list` / branch explain. Kept short (not a full fetch +// budget): discovery is best-effort and additive — on timeout or unreachable +// remote the store falls back to local refs rather than stalling a previously +// instant command for tens of seconds. +const checkpointRefListTimeout = 5 * time.Second // ListCheckpointRefsOnRemote enumerates the per-checkpoint refs // (refs/entire/checkpoints//) present on the checkpoint remote, names @@ -510,12 +513,20 @@ const checkpointRefListTimeout = 30 * time.Second // remote, and with none configured it returns (nil, nil) so List stays // local-only rather than scanning origin. This keeps the default (no // checkpoint_remote) behavior unchanged. +// +// Resolution and ls-remote are pinned to the worktree root (not process cwd) so +// repo-local git config (url.*.insteadOf, credential helpers, remotes) applies. func ListCheckpointRefsOnRemote(ctx context.Context) ([]plumbing.ReferenceName, error) { if !remote.Configured(ctx) { return nil, nil } - url, err := remote.FetchURL(ctx) + worktreeRoot, err := paths.WorktreeRoot(ctx) + if err != nil { + return nil, fmt.Errorf("resolve worktree root: %w", err) + } + + url, err := remote.FetchURL(ctx, remote.FetchURLOptions{WorktreeRoot: worktreeRoot}) if err != nil { return nil, fmt.Errorf("resolve checkpoint remote URL: %w", err) } @@ -523,7 +534,7 @@ func ListCheckpointRefsOnRemote(ctx context.Context) ([]plumbing.ReferenceName, ctx, cancel := context.WithTimeout(ctx, checkpointRefListTimeout) defer cancel() - output, err := remote.LsRemoteInDir(ctx, "", url, checkpoint.CheckpointRefPrefix+"*") + output, err := remote.LsRemoteInDir(ctx, worktreeRoot, url, checkpoint.CheckpointRefPrefix+"*") if err != nil { return nil, fmt.Errorf("ls-remote checkpoint refs from %s: %w", remote.RedactURL(url), err) } diff --git a/cmd/entire/cli/git_operations_test.go b/cmd/entire/cli/git_operations_test.go index 2f6c502f6e..c100f423bd 100644 --- a/cmd/entire/cli/git_operations_test.go +++ b/cmd/entire/cli/git_operations_test.go @@ -7,6 +7,7 @@ import ( "path/filepath" "strings" "testing" + "time" "github.com/entireio/cli/cmd/entire/cli/checkpoint" "github.com/entireio/cli/cmd/entire/cli/checkpoint/remote" @@ -716,3 +717,32 @@ func TestListCheckpointRefsOnRemote_NotConfigured(t *testing.T) { require.NoError(t, err) assert.Nil(t, names, "no checkpoint_remote configured must leave List local-only (no remote enumeration)") } + +// TestListCheckpointRefsOnRemote_ResolvesFromSubdir proves worktree pinning: +// enumeration resolves the worktree root (and therefore repo-local settings) +// even when the process cwd is a subdirectory, rather than depending on cwd +// alone. Not parallel: uses t.Chdir. +func TestListCheckpointRefsOnRemote_ResolvesFromSubdir(t *testing.T) { + dir := t.TempDir() + testutil.InitRepo(t, dir) + testutil.WriteFile(t, dir, "f.txt", "init") + testutil.GitAdd(t, dir, "f.txt") + testutil.GitCommit(t, dir, "init") + + sub := filepath.Join(dir, "nested", "deep") + require.NoError(t, os.MkdirAll(sub, 0o755)) + t.Chdir(sub) + + names, err := ListCheckpointRefsOnRemote(context.Background()) + require.NoError(t, err) + assert.Nil(t, names, "no checkpoint_remote from a subdir must still be a no-op (worktree-resolved settings)") +} + +// TestCheckpointRefListTimeoutIsShort pins the discovery budget: user-facing +// list/explain must not inherit a long fetch-style hang when the remote is +// unreachable (best-effort fallback to local refs). +func TestCheckpointRefListTimeoutIsShort(t *testing.T) { + t.Parallel() + assert.LessOrEqual(t, checkpointRefListTimeout, 5*time.Second) + assert.Greater(t, checkpointRefListTimeout, time.Duration(0)) +} diff --git a/docs/architecture/ref-checkpoint-backend.md b/docs/architecture/ref-checkpoint-backend.md index 8d5bc0382e..d12cc4fc8d 100644 --- a/docs/architecture/ref-checkpoint-backend.md +++ b/docs/architecture/ref-checkpoint-backend.md @@ -128,7 +128,7 @@ To close that gap `List` supports **opt-in remote discovery**: - A **remote ref lister** injected by the CLI runs a single `git ls-remote refs/entire/checkpoints/*` — **names only, no object transfer** — against the checkpoint remote. - Each advertised ref that has no local ref yet is added to the result as a not-yet-hydrated `CheckpointInfo`. Its `CreatedAt` is recovered from the ULID timestamp in the ref name, so it sorts by real recency without a fetch; the rest of its contents are **hydrated lazily on the next read** via the on-demand ref fetch. -Enumeration is **checkpoint-remote-scoped** — the same authority rule as the on-demand fetch and #1719: with a `checkpoint_remote` configured it queries that remote, and with **none** configured it does nothing (returns no refs), leaving `List` local-only rather than scanning origin. Discovery is also **best-effort and additive**: an `ls-remote` failure logs and returns the local results rather than failing the whole listing. +Enumeration is **checkpoint-remote-scoped** — the same authority rule as the on-demand fetch and #1719: with a `checkpoint_remote` configured it queries that remote, and with **none** configured it does nothing (returns no refs), leaving `List` local-only rather than scanning origin. Discovery is also **best-effort and additive**: an `ls-remote` failure or a short timeout (a few seconds — this path must not turn a previously-local listing into a long hang when the remote is unreachable) logs and returns the local results rather than failing the whole listing. URL resolution and `ls-remote` run from the worktree root so repo-local git config applies. ## Read routing and coexistence From 9dda4a8d341c7477c0c8602c807e24df188a3dd0 Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Thu, 16 Jul 2026 09:33:18 -0400 Subject: [PATCH 3/6] fix(checkpoint): hydrate remote List stubs before --session filter Names-only remote-discovered CheckpointInfos left SessionID empty, so `entire checkpoint list --session` silently dropped the second-device checkpoints this path is meant to surface. Hydrate on collect (and match archived SessionIDs in the human list filter). Co-authored-by: Cursor --- cmd/entire/cli/checkpoint/refs_store.go | 50 ++++++++++++++++++++ cmd/entire/cli/checkpoint/refs_store_test.go | 35 ++++++++++++++ cmd/entire/cli/explain.go | 13 ++++- cmd/entire/cli/explain_test.go | 39 +++++++++++++++ 4 files changed, 135 insertions(+), 2 deletions(-) diff --git a/cmd/entire/cli/checkpoint/refs_store.go b/cmd/entire/cli/checkpoint/refs_store.go index 9e5a471823..09073d987f 100644 --- a/cmd/entire/cli/checkpoint/refs_store.go +++ b/cmd/entire/cli/checkpoint/refs_store.go @@ -483,6 +483,56 @@ func remoteDiscoveredInfo(cid id.CheckpointID) CheckpointInfo { return info } +// listedCheckpointNeedsHydration reports whether info is a names-only List stub +// (as produced by remoteDiscoveredInfo): CheckpointID/CreatedAt may be set, but +// SessionID and SessionCount are still zero. Callers that need session identity +// for filtering or display should HydrateListedCheckpointInfo first. +func listedCheckpointNeedsHydration(info CheckpointInfo) bool { + return info.SessionID == "" && info.SessionCount == 0 && !info.CheckpointID.IsEmpty() +} + +// HydrateListedCheckpointInfo fills SessionID/Agent/etc for a List entry that +// was discovered by name only. It reads the checkpoint (triggering an on-demand +// ref fetch when configured) and mirrors the fields List populates for local +// refs. Best-effort: returns info unchanged on read failure so listing still +// surfaces the checkpoint without session metadata. +func HydrateListedCheckpointInfo(ctx context.Context, store interface { + Read(ctx context.Context, checkpointID id.CheckpointID) (*CheckpointSummary, error) + ReadSessionMetadata(ctx context.Context, checkpointID id.CheckpointID, sessionIndex int) (*Metadata, error) +}, info CheckpointInfo) CheckpointInfo { + if !listedCheckpointNeedsHydration(info) { + return info + } + summary, err := store.Read(ctx, info.CheckpointID) + if err != nil || summary == nil { + return info + } + info.CheckpointsCount = summary.CheckpointsCount + info.FilesTouched = summary.FilesTouched + info.SessionCount = len(summary.Sessions) + info.Imported = summary.Imported + info.SessionIDs = info.SessionIDs[:0] + for i := range summary.Sessions { + meta, metaErr := store.ReadSessionMetadata(ctx, info.CheckpointID, i) + if metaErr != nil || meta == nil { + continue + } + if meta.SessionID != "" { + info.SessionIDs = append(info.SessionIDs, meta.SessionID) + } + if i == len(summary.Sessions)-1 { + info.Agent = meta.Agent + info.SessionID = meta.SessionID + if !meta.CreatedAt.IsZero() { + info.CreatedAt = meta.CreatedAt + } + info.IsTask = meta.IsTask + info.ToolUseID = meta.ToolUseID + } + } + return info +} + // GetCheckpointAuthor returns the author of the checkpoint ref's tip commit (the // most recent writer). Returns a zero Author when the ref is absent. func (s *gitRefsStore) GetCheckpointAuthor(ctx context.Context, checkpointID id.CheckpointID) (Author, error) { diff --git a/cmd/entire/cli/checkpoint/refs_store_test.go b/cmd/entire/cli/checkpoint/refs_store_test.go index ded9021fbb..9362f90a40 100644 --- a/cmd/entire/cli/checkpoint/refs_store_test.go +++ b/cmd/entire/cli/checkpoint/refs_store_test.go @@ -230,6 +230,41 @@ func TestGitRefsStore_ListRemoteDiscovery(t *testing.T) { }) } +// TestHydrateListedCheckpointInfo covers the trail-871 gap: a names-only List +// stub has empty SessionID, so --session filters would silently drop it until +// the checkpoint is read. HydrateListedCheckpointInfo fills session identity +// from the store (triggering on-demand fetch when configured) so filters match. +func TestHydrateListedCheckpointInfo(t *testing.T) { + t.Parallel() + + store := newRefsStore(t) + cid := id.MustCheckpointID("a1b2c3d4e5f6") + refsWrite(t, store, cid, "session-from-device-a", "transcript") + + stub := remoteDiscoveredInfo(cid) + require.True(t, listedCheckpointNeedsHydration(stub)) + require.Empty(t, stub.SessionID) + require.Zero(t, stub.SessionCount) + + hydrated := HydrateListedCheckpointInfo(context.Background(), store, stub) + assert.Equal(t, "session-from-device-a", hydrated.SessionID) + assert.Equal(t, 1, hydrated.SessionCount) + assert.Equal(t, []string{"session-from-device-a"}, hydrated.SessionIDs) + assert.False(t, listedCheckpointNeedsHydration(hydrated)) + + // Already-hydrated infos are returned unchanged (no redundant reads needed + // for the session-filter path once collectCheckpoint has cached them). + again := HydrateListedCheckpointInfo(context.Background(), store, hydrated) + assert.Equal(t, hydrated, again) + + // Missing checkpoint: best-effort leave the stub as-is so listing still + // surfaces the ID even when the remote fetch/read fails. + missing := remoteDiscoveredInfo(id.CheckpointID("01KVBJCWYA4YW6J5M9GP655HZN")) + unchanged := HydrateListedCheckpointInfo(context.Background(), store, missing) + assert.Equal(t, missing, unchanged) + assert.Empty(t, unchanged.SessionID) +} + func TestGitRefsStore_WriteAllVariantsAndRead(t *testing.T) { t.Parallel() store := newRefsStore(t) diff --git a/cmd/entire/cli/explain.go b/cmd/entire/cli/explain.go index 5bc8b65d3c..e31f469e1a 100644 --- a/cmd/entire/cli/explain.go +++ b/cmd/entire/cli/explain.go @@ -2097,6 +2097,12 @@ func getBranchCheckpoints(ctx context.Context, repo *git.Repository, limit int) if !found { return } + // Remote-discovered refs-native List stubs carry only CheckpointID/ + // CreatedAt. Hydrate before projecting into RewindPoint so --session + // filters (and prompt display) see the real SessionID instead of + // silently dropping the second-device checkpoints this path surfaces. + cpInfo = checkpoint.HydrateListedCheckpointInfo(ctx, store, cpInfo) + committedByID[cpID] = cpInfo message := strings.Split(c.Message, "\n")[0] point := strategy.RewindPoint{ @@ -2586,11 +2592,14 @@ func formatBranchCheckpoints(w io.Writer, branchName string, points []strategy.R var sb strings.Builder styles := newStatusStyles(w) - // Filter by session if specified (must happen before counting) + // Filter by session if specified (must happen before counting). Use the + // shared matcher so archived contributors in SessionIDs are considered — + // matching only SessionID (latest contributor) silently drops multi-session + // checkpoints where the requested session was archived. if sessionFilter != "" { var filtered []strategy.RewindPoint for _, p := range points { - if p.SessionID == sessionFilter || strings.HasPrefix(p.SessionID, sessionFilter) { + if checkpointMatchesSessionFilter(p, sessionFilter) { filtered = append(filtered, p) } } diff --git a/cmd/entire/cli/explain_test.go b/cmd/entire/cli/explain_test.go index dd5af7de7c..22cd0acb08 100644 --- a/cmd/entire/cli/explain_test.go +++ b/cmd/entire/cli/explain_test.go @@ -4230,6 +4230,45 @@ func TestFormatBranchCheckpoints_SessionFilter(t *testing.T) { t.Errorf("expected 'session ... nonexistent-session' in output, got:\n%s", output) } }) + + t.Run("filter matches archived SessionIDs contributor", func(t *testing.T) { + // Multi-session checkpoint: latest SessionID is beta, but alpha is still + // in SessionIDs. The shared matcher must keep it when filtering for alpha. + multi := []strategy.RewindPoint{ + { + ID: "abc123def456", + Message: "multi-session checkpoint", + Date: now, + CheckpointID: "chk444444444", + SessionID: "2026-01-22-session-beta", + SessionIDs: []string{"2026-01-22-session-alpha", "2026-01-22-session-beta"}, + }, + } + output := formatBranchCheckpoints(io.Discard, "main", multi, "2026-01-22-session-alpha") + if !strings.Contains(output, "checkpoints 1") { + t.Errorf("expected archived contributor to match session filter, got:\n%s", output) + } + }) + + t.Run("unhydrated remote stub does not match session filter", func(t *testing.T) { + // Documents the pre-hydrate failure mode trail 871 caught: a names-only + // List stub has empty SessionID, so --session would drop it. Production + // collectCheckpoint hydrates before formatting; this asserts the filter + // itself does not invent a match for an empty SessionID. + stub := []strategy.RewindPoint{ + { + ID: "abc123def456", + Message: "remote-discovered stub", + Date: now, + CheckpointID: "01KVBJCWYA4YW6J5M9GP655HZN", + SessionID: "", + }, + } + output := formatBranchCheckpoints(io.Discard, "main", stub, "2026-01-22-session-alpha") + if !strings.Contains(output, "checkpoints 0") { + t.Errorf("empty SessionID stub must not match a session filter, got:\n%s", output) + } + }) } func TestRunExplain_SessionFlagFiltersListView(t *testing.T) { From 0a4e2cd16bbaa526f675b554f2f43868ea4f3352 Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Tue, 21 Jul 2026 19:46:12 -0400 Subject: [PATCH 4/6] fix(checkpoint): address trail 871 remote list discovery review Hydrate stubs after display-limit truncate with a short per-ref budget and fail-once latch, surface discovery/hydration failures, and correct authority docs. Add offline getBranchCheckpoints coverage and tighten weak list tests. Co-authored-by: Cursor --- api/checkpoint/metadata.go | 8 ++ cmd/entire/cli/checkpoint/fetching_tree.go | 10 +- cmd/entire/cli/checkpoint/refs_naming.go | 4 +- cmd/entire/cli/checkpoint/refs_store.go | 94 +++++++++++---- cmd/entire/cli/checkpoint/refs_store_test.go | 44 ++++++- cmd/entire/cli/checkpoint/remote/git.go | 24 +++- cmd/entire/cli/explain.go | 67 +++++++++-- .../cli/explain_remote_discovery_test.go | 112 ++++++++++++++++++ cmd/entire/cli/git_operations.go | 18 +-- cmd/entire/cli/git_operations_test.go | 70 ++++++++--- docs/architecture/ref-checkpoint-backend.md | 6 +- 11 files changed, 394 insertions(+), 63 deletions(-) create mode 100644 cmd/entire/cli/explain_remote_discovery_test.go diff --git a/api/checkpoint/metadata.go b/api/checkpoint/metadata.go index 5a39e7ecd7..e6328b78ab 100644 --- a/api/checkpoint/metadata.go +++ b/api/checkpoint/metadata.go @@ -293,6 +293,14 @@ type CheckpointInfo struct { // Imported is true when this checkpoint was imported from pre-existing // agent history (Kind == "imported"): read-only and commit-less. Imported bool + + // ListedStub is true for names-only remote-discovery List entries that still + // need hydration (or have not yet failed a hydration attempt). It is cleared + // after a successful hydrate and also after a failed attempt (fail-once), so + // callers do not re-fetch forever. A local ref whose root metadata was + // unreadable has the same zero SessionID/SessionCount shape but ListedStub + // false — do not treat field zero-ness alone as stub-ness. + ListedStub bool `json:"-"` } // SessionContent contains the actual content for a session. diff --git a/cmd/entire/cli/checkpoint/fetching_tree.go b/cmd/entire/cli/checkpoint/fetching_tree.go index d83dd4171d..a24fa33410 100644 --- a/cmd/entire/cli/checkpoint/fetching_tree.go +++ b/cmd/entire/cli/checkpoint/fetching_tree.go @@ -29,9 +29,13 @@ type RefFetchFunc func(ctx context.Context, ref plumbing.ReferenceName) error // in List to discover checkpoints written on another machine that have no local // ref yet; each discovered checkpoint is then hydrated lazily on read via // RefFetchFunc. The checkpoint package cannot resolve the remote target itself, -// so the CLI layer injects it. Enumeration is checkpoint-remote-scoped (never -// origin): it returns (nil, nil) when no checkpoint remote is configured, which -// leaves List local-only — the same authority rule as the on-demand fetch. +// so the CLI layer injects it. +// +// Scope is stricter than the on-demand read fetch: with no checkpoint_remote +// configured the lister returns (nil, nil) and List stays local-only. The +// on-demand fetch (FetchURL) falls back to origin in that case. When a +// checkpoint_remote is configured the lister queries the resolved checkpoint +// URL (which can still fall through to origin in FetchURL edge cases). type RemoteRefListFunc func(ctx context.Context) ([]plumbing.ReferenceName, error) // FetchingTree wraps a git tree to automatically fetch missing blobs on demand. diff --git a/cmd/entire/cli/checkpoint/refs_naming.go b/cmd/entire/cli/checkpoint/refs_naming.go index aac115026a..09929a70ca 100644 --- a/cmd/entire/cli/checkpoint/refs_naming.go +++ b/cmd/entire/cli/checkpoint/refs_naming.go @@ -17,8 +17,8 @@ const CheckpointRefPrefix = "refs/entire/checkpoints/" // RefName returns the per-checkpoint git ref for a checkpoint ID: // refs/entire/checkpoints//, where is id.ShardFor() (the -// first two chars for legacy hex IDs, the last two for ULIDs). The full ID is -// always the leaf, so the ref round-trips through ParseRef. +// last two characters of the ID for both legacy hex and ULID formats). The full +// ID is always the leaf, so the ref round-trips through ParseRef. // // It errors on an empty or unrecognized checkpoint ID rather than returning a // malformed ref (e.g. "refs/entire/checkpoints//"), so callers at trust diff --git a/cmd/entire/cli/checkpoint/refs_store.go b/cmd/entire/cli/checkpoint/refs_store.go index 09073d987f..76a497f133 100644 --- a/cmd/entire/cli/checkpoint/refs_store.go +++ b/cmd/entire/cli/checkpoint/refs_store.go @@ -5,7 +5,9 @@ import ( "errors" "fmt" "log/slog" + "os" "strconv" + "time" "github.com/go-git/go-git/v6" "github.com/go-git/go-git/v6/plumbing" @@ -16,6 +18,12 @@ import ( "github.com/entireio/cli/cmd/entire/cli/validation" ) +// ListHydrationTimeout is the per-ref budget for hydrating names-only List stubs +// during user-facing enumeration (after the display-limit truncate). Shorter than +// the default on-demand fetch budget so a stuck remote cannot turn list/explain +// into many minutes of sequential ref fetches. +const ListHydrationTimeout = 15 * time.Second + var ( _ PersistentStore = (*gitRefsStore)(nil) _ AuthorReader = (*gitRefsStore)(nil) @@ -448,13 +456,16 @@ func (s *gitRefsStore) List(ctx context.Context) ([]CheckpointInfo, error) { // not-yet-hydrated CheckpointInfos. It never fetches objects: the ref name // yields the checkpoint ID, and a ULID ID yields its creation time, so a // discovered checkpoint sorts and displays correctly before its first read -// hydrates the rest. Best-effort: an enumeration failure logs and returns the -// unchanged local list. +// hydrates the rest. Best-effort: an enumeration failure logs, warns on stderr, +// and returns the unchanged local list. func (s *gitRefsStore) appendRemoteDiscovered(ctx context.Context, checkpoints []CheckpointInfo, seen map[id.CheckpointID]struct{}) []CheckpointInfo { remoteRefs, err := s.remoteRefLister(ctx) if err != nil { logging.Warn(ctx, "git-refs: remote checkpoint enumeration failed; listing local refs only", slog.String("error", err.Error())) + // Match WarnIfMetadataDisconnected: opted-in discovery failing must be + // visible on stderr — logging.Warn alone lands only in .entire/logs/. + fmt.Fprintln(os.Stderr, "[entire] Warning: could not reach checkpoint remote; showing local checkpoints only.") return checkpoints } for _, refName := range remoteRefs { @@ -475,8 +486,10 @@ func (s *gitRefsStore) appendRemoteDiscovered(ctx context.Context, checkpoints [ // only by its remote ref name. Its contents are not fetched here (that happens // lazily on read); CreatedAt is recovered from the ULID timestamp so the entry // sorts by real creation time, and is left zero for a (rare) legacy-hex ref. +// ListedStub marks the entry so hydration can distinguish it from a local ref +// whose root metadata was unreadable (same zero SessionID/SessionCount shape). func remoteDiscoveredInfo(cid id.CheckpointID) CheckpointInfo { - info := CheckpointInfo{CheckpointID: cid} + info := CheckpointInfo{CheckpointID: cid, ListedStub: true} if createdAt, ok := cid.Time(); ok { info.CreatedAt = createdAt } @@ -484,18 +497,28 @@ func remoteDiscoveredInfo(cid id.CheckpointID) CheckpointInfo { } // listedCheckpointNeedsHydration reports whether info is a names-only List stub -// (as produced by remoteDiscoveredInfo): CheckpointID/CreatedAt may be set, but -// SessionID and SessionCount are still zero. Callers that need session identity -// for filtering or display should HydrateListedCheckpointInfo first. +// that still needs a hydrate attempt. It keys off ListedStub (set by +// remoteDiscoveredInfo), not SessionID/SessionCount zero-ness: a local ref whose +// root metadata.json is missing/unreadable has the same zero fields but must not +// be treated as a stub (hydration can never fix it and would re-fetch forever). +// Callers that need session identity for filtering or display should +// HydrateListedCheckpointInfo first. func listedCheckpointNeedsHydration(info CheckpointInfo) bool { - return info.SessionID == "" && info.SessionCount == 0 && !info.CheckpointID.IsEmpty() + return info.ListedStub && !info.CheckpointID.IsEmpty() } // HydrateListedCheckpointInfo fills SessionID/Agent/etc for a List entry that // was discovered by name only. It reads the checkpoint (triggering an on-demand // ref fetch when configured) and mirrors the fields List populates for local -// refs. Best-effort: returns info unchanged on read failure so listing still -// surfaces the checkpoint without session metadata. +// refs via readCommittedInfoFromCheckpointTree, with one deliberate CreatedAt +// divergence: the local List path assigns info.CreatedAt = meta.CreatedAt +// unconditionally, while hydration only overwrites when meta.CreatedAt is +// non-zero (keeping the ULID-derived time from remoteDiscoveredInfo). +// +// Best-effort / fail-once: on Read or last-session metadata failure it logs Warn, +// clears ListedStub so callers do not re-fetch, and returns the original stub +// fields (never a half-hydrated SessionCount-without-SessionID that would poison +// a committedByID cache and still look "done"). func HydrateListedCheckpointInfo(ctx context.Context, store interface { Read(ctx context.Context, checkpointID id.CheckpointID) (*CheckpointSummary, error) ReadSessionMetadata(ctx context.Context, checkpointID id.CheckpointID, sessionIndex int) (*Metadata, error) @@ -505,32 +528,61 @@ func HydrateListedCheckpointInfo(ctx context.Context, store interface { } summary, err := store.Read(ctx, info.CheckpointID) if err != nil || summary == nil { + logging.Warn(ctx, "git-refs: failed to hydrate remote-discovered checkpoint; leaving stub without session metadata", + slog.String("checkpoint_id", info.CheckpointID.String()), + slog.String("error", errString(err))) + info.ListedStub = false // fail-once: do not re-fetch on every list return info } - info.CheckpointsCount = summary.CheckpointsCount - info.FilesTouched = summary.FilesTouched - info.SessionCount = len(summary.Sessions) - info.Imported = summary.Imported - info.SessionIDs = info.SessionIDs[:0] + + out := info + out.ListedStub = false + out.CheckpointsCount = summary.CheckpointsCount + out.FilesTouched = summary.FilesTouched + out.SessionCount = len(summary.Sessions) + out.Imported = summary.Imported + out.SessionIDs = nil + lastMetaOK := len(summary.Sessions) == 0 for i := range summary.Sessions { meta, metaErr := store.ReadSessionMetadata(ctx, info.CheckpointID, i) if metaErr != nil || meta == nil { + logging.Warn(ctx, "git-refs: failed to read session metadata while hydrating remote-discovered checkpoint", + slog.String("checkpoint_id", info.CheckpointID.String()), + slog.Int("session_index", i), + slog.String("error", errString(metaErr))) continue } if meta.SessionID != "" { - info.SessionIDs = append(info.SessionIDs, meta.SessionID) + out.SessionIDs = append(out.SessionIDs, meta.SessionID) } if i == len(summary.Sessions)-1 { - info.Agent = meta.Agent - info.SessionID = meta.SessionID + out.Agent = meta.Agent + out.SessionID = meta.SessionID if !meta.CreatedAt.IsZero() { - info.CreatedAt = meta.CreatedAt + out.CreatedAt = meta.CreatedAt } - info.IsTask = meta.IsTask - info.ToolUseID = meta.ToolUseID + out.IsTask = meta.IsTask + out.ToolUseID = meta.ToolUseID + lastMetaOK = true } } - return info + if !lastMetaOK { + // Avoid caching SessionCount>0 with empty SessionID: that shape no longer + // needs hydration under the old zero-field heuristic and would poison + // committedByID / --session filters. Fail-once on the original stub. + logging.Warn(ctx, "git-refs: remote-discovered checkpoint hydration incomplete; leaving stub without session metadata", + slog.String("checkpoint_id", info.CheckpointID.String())) + info.ListedStub = false + return info + } + return out +} + +func errString(err error) string { + if err == nil { + return "nil result" + } + return err.Error() } // GetCheckpointAuthor returns the author of the checkpoint ref's tip commit (the diff --git a/cmd/entire/cli/checkpoint/refs_store_test.go b/cmd/entire/cli/checkpoint/refs_store_test.go index 9362f90a40..2d68c066ad 100644 --- a/cmd/entire/cli/checkpoint/refs_store_test.go +++ b/cmd/entire/cli/checkpoint/refs_store_test.go @@ -243,6 +243,7 @@ func TestHydrateListedCheckpointInfo(t *testing.T) { stub := remoteDiscoveredInfo(cid) require.True(t, listedCheckpointNeedsHydration(stub)) + require.True(t, stub.ListedStub) require.Empty(t, stub.SessionID) require.Zero(t, stub.SessionCount) @@ -251,18 +252,51 @@ func TestHydrateListedCheckpointInfo(t *testing.T) { assert.Equal(t, 1, hydrated.SessionCount) assert.Equal(t, []string{"session-from-device-a"}, hydrated.SessionIDs) assert.False(t, listedCheckpointNeedsHydration(hydrated)) + assert.False(t, hydrated.ListedStub) // Already-hydrated infos are returned unchanged (no redundant reads needed // for the session-filter path once collectCheckpoint has cached them). again := HydrateListedCheckpointInfo(context.Background(), store, hydrated) assert.Equal(t, hydrated, again) - // Missing checkpoint: best-effort leave the stub as-is so listing still - // surfaces the ID even when the remote fetch/read fails. + // Missing checkpoint: fail-once clears ListedStub so callers do not re-fetch, + // but leaves SessionID empty so listing can still surface the ID. missing := remoteDiscoveredInfo(id.CheckpointID("01KVBJCWYA4YW6J5M9GP655HZN")) - unchanged := HydrateListedCheckpointInfo(context.Background(), store, missing) - assert.Equal(t, missing, unchanged) - assert.Empty(t, unchanged.SessionID) + failed := HydrateListedCheckpointInfo(context.Background(), store, missing) + assert.Equal(t, missing.CheckpointID, failed.CheckpointID) + assert.Empty(t, failed.SessionID) + assert.False(t, failed.ListedStub, "failed hydration must clear ListedStub (fail-once)") + assert.False(t, listedCheckpointNeedsHydration(failed)) +} + +// TestHydrateListedCheckpointInfo_MatchesLocalList pins the field mapping shared +// with readCommittedInfoFromCheckpointTree: hydrating a stub for a locally +// present checkpoint must yield the same CheckpointInfo that List returns for +// it. Deliberate CreatedAt divergence (documented on HydrateListedCheckpointInfo): +// local List assigns meta.CreatedAt unconditionally; hydration only overwrites +// when non-zero (keeping ULID-derived time). A normal refsWrite has non-zero +// CreatedAt, so both paths agree here. +func TestHydrateListedCheckpointInfo_MatchesLocalList(t *testing.T) { + t.Parallel() + + store := newRefsStore(t) + cid := id.MustCheckpointID("a1b2c3d4e5f6") + refsWrite(t, store, cid, "session-from-device-a", "transcript") + + infos, err := store.List(context.Background()) + require.NoError(t, err) + var local CheckpointInfo + for _, info := range infos { + if info.CheckpointID == cid { + local = info + break + } + } + require.Equal(t, cid, local.CheckpointID) + require.False(t, local.ListedStub) + + hydrated := HydrateListedCheckpointInfo(context.Background(), store, remoteDiscoveredInfo(cid)) + assert.Equal(t, local, hydrated) } func TestGitRefsStore_WriteAllVariantsAndRead(t *testing.T) { diff --git a/cmd/entire/cli/checkpoint/remote/git.go b/cmd/entire/cli/checkpoint/remote/git.go index 1de3ef55fb..be9795ad91 100644 --- a/cmd/entire/cli/checkpoint/remote/git.go +++ b/cmd/entire/cli/checkpoint/remote/git.go @@ -3,6 +3,7 @@ package remote import ( "context" "encoding/base64" + "errors" "fmt" "log/slog" "os" @@ -461,11 +462,32 @@ func lsRemote(ctx context.Context, dir, remote string, patterns ...string) ([]by disableTerminalPrompt(cmd) out, err := cmd.Output() if err != nil { - return out, fmt.Errorf("git ls-remote: %w", err) + return out, fmt.Errorf("git ls-remote: %w", formatGitCommandError(ctx, err)) } return out, nil } +// formatGitCommandError enriches an exec error from git Output() so callers see +// useful detail: context deadline expiry by name, and git's stderr (auth denied, +// repository not found, DNS) which ExitError otherwise hides behind "exit status N". +func formatGitCommandError(ctx context.Context, err error) error { + if err == nil { + return nil + } + if errors.Is(ctx.Err(), context.DeadlineExceeded) { + return fmt.Errorf("deadline exceeded: %w", err) + } + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + if stderr := strings.TrimSpace(string(exitErr.Stderr)); stderr != "" { + // Collapse whitespace so multi-line git stderr stays one log/attr value. + stderr = strings.Join(strings.Fields(stderr), " ") + return fmt.Errorf("%w (%s)", err, stderr) + } + } + return err +} + // IsURL returns true if the target looks like a URL rather than a git remote name. func IsURL(target string) bool { return strings.Contains(target, "://") || strings.Contains(target, "@") diff --git a/cmd/entire/cli/explain.go b/cmd/entire/cli/explain.go index 37415c5c77..2eb3ff6f80 100644 --- a/cmd/entire/cli/explain.go +++ b/cmd/entire/cli/explain.go @@ -2140,12 +2140,11 @@ func getBranchCheckpoints(ctx context.Context, repo *git.Repository, limit int) if !found { return } - // Remote-discovered refs-native List stubs carry only CheckpointID/ - // CreatedAt. Hydrate before projecting into RewindPoint so --session - // filters (and prompt display) see the real SessionID instead of - // silently dropping the second-device checkpoints this path surfaces. - cpInfo = checkpoint.HydrateListedCheckpointInfo(ctx, store, cpInfo) - committedByID[cpID] = cpInfo + // Defer hydration of remote-discovered stubs until after sort+truncate + // below: hydrating here (during the commitScanLimit walk) can issue up + // to hundreds of sequential ref fetches to display at most `limit` + // entries. Stubs project with empty SessionID; hydrateListedBranchCheckpoints + // fills them before --session filters run. message := strings.Split(c.Message, "\n")[0] point := strategy.RewindPoint{ @@ -2161,7 +2160,9 @@ func getBranchCheckpoints(ctx context.Context, repo *git.Repository, limit int) ToolUseID: cpInfo.ToolUseID, Agent: cpInfo.Agent, } - point.SessionPrompt = readLatestCommittedSessionPrompt(ctx, store, cpID, cpInfo.SessionCount) + if !cpInfo.ListedStub { + point.SessionPrompt = readLatestCommittedSessionPrompt(ctx, store, cpID, cpInfo.SessionCount) + } points = append(points, point) } @@ -2226,6 +2227,10 @@ func getBranchCheckpoints(ctx context.Context, repo *git.Repository, limit int) truncated = true } + // Hydrate remote-discovered stubs only for the truncated display set (not + // the full commit walk). Session filter runs later in formatBranchCheckpoints. + hydrateListedBranchCheckpoints(ctx, store, points, committedByID) + // Append imported (read-only, commit-less) checkpoints after the live points, // bounded by the same limit so a one-month import doesn't produce an // unbounded list. They get their own budget and never displace live points. @@ -2242,6 +2247,54 @@ func getBranchCheckpoints(ctx context.Context, repo *git.Repository, limit int) return points, truncated, nil } +// hydrateListedBranchCheckpoints fills SessionID/etc for remote-discovered List +// stubs among the already-truncated RewindPoints. Per-ref fetch budget is +// ListHydrationTimeout (much shorter than the default on-demand fetch). Failures +// clear ListedStub (fail-once) inside HydrateListedCheckpointInfo; when any +// stub still lacks SessionID afterward we note it on stderr so --session +// filters dropping them is not silent. +func hydrateListedBranchCheckpoints( + ctx context.Context, + store interface { + checkpoint.SessionReader + Read(ctx context.Context, checkpointID id.CheckpointID) (*checkpoint.CheckpointSummary, error) + ReadSessionMetadata(ctx context.Context, checkpointID id.CheckpointID, sessionIndex int) (*checkpoint.Metadata, error) + }, + points []strategy.RewindPoint, + committedByID map[id.CheckpointID]checkpoint.CheckpointInfo, +) { + hydrationFailed := 0 + for i := range points { + cpID := points[i].CheckpointID + if cpID.IsEmpty() { + continue + } + cpInfo, ok := committedByID[cpID] + if !ok || !cpInfo.ListedStub { + continue + } + hctx, cancel := context.WithTimeout(ctx, checkpoint.ListHydrationTimeout) + hydrated := checkpoint.HydrateListedCheckpointInfo(hctx, store, cpInfo) + cancel() + committedByID[cpID] = hydrated + points[i].SessionID = hydrated.SessionID + points[i].SessionCount = hydrated.SessionCount + points[i].SessionIDs = hydrated.SessionIDs + points[i].IsTaskCheckpoint = hydrated.IsTask + points[i].ToolUseID = hydrated.ToolUseID + points[i].Agent = hydrated.Agent + if hydrated.SessionCount > 0 { + points[i].SessionPrompt = readLatestCommittedSessionPrompt(ctx, store, cpID, hydrated.SessionCount) + } + if hydrated.SessionID == "" { + hydrationFailed++ + } + } + if hydrationFailed > 0 { + fmt.Fprintf(os.Stderr, "[entire] Warning: could not load session metadata for %d remote checkpoint(s); they may be missing from --session filters.\n", hydrationFailed) + } +} + // getImportedRewindPoints returns read-only imported checkpoints (Kind // "imported", flagged Imported) as RewindPoint entries. They live on the v1 // metadata branch but carry no commit trailer, so the commit-driven branch diff --git a/cmd/entire/cli/explain_remote_discovery_test.go b/cmd/entire/cli/explain_remote_discovery_test.go new file mode 100644 index 0000000000..c90f6f5bc7 --- /dev/null +++ b/cmd/entire/cli/explain_remote_discovery_test.go @@ -0,0 +1,112 @@ +package cli + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/entireio/cli/cmd/entire/cli/checkpoint" + "github.com/entireio/cli/cmd/entire/cli/checkpoint/id" + "github.com/entireio/cli/cmd/entire/cli/testutil" + "github.com/entireio/cli/cmd/entire/cli/trailers" + "github.com/entireio/cli/redact" + "github.com/go-git/go-git/v6" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestGetBranchCheckpoints_HydratesRemoteDiscoveredStub is the trail-871 / +// PR-1771 headline integration: device B has the trailer commit (pulled) but no +// local checkpoint ref; with checkpoint_remote configured against a local bare +// remote that advertises the ref, getBranchCheckpoints must return a RewindPoint +// with the real SessionID (not an empty stub that --session would drop). +// +// Offline: provider "local" is unknown to providerHost, so FetchURL falls back +// to origin after file:// derivation fails — origin is the bare file:// URL. +// Not parallel: uses t.Chdir. +func TestGetBranchCheckpoints_HydratesRemoteDiscoveredStub(t *testing.T) { + bareDir := t.TempDir() + gitRun(t, bareDir, "init", "--bare", "-q", bareDir) + bareURL := "file://" + filepath.ToSlash(bareDir) + + deviceA := t.TempDir() + testutil.InitRepo(t, deviceA) + testutil.WriteFile(t, deviceA, "f.txt", "init") + testutil.GitAdd(t, deviceA, "f.txt") + testutil.GitCommit(t, deviceA, "init") + branch := gitDefaultBranch(t, deviceA) + gitRun(t, deviceA, "remote", "add", "origin", bareURL) + gitRun(t, deviceA, "push", "-q", "-u", "origin", "HEAD:"+branch) + gitRun(t, bareDir, "symbolic-ref", "HEAD", "refs/heads/"+branch) + + settingsBody := `{"enabled":true,"checkpoints":{"primary":{"type":"git-refs"}},"strategy_options":{"checkpoint_remote":{"provider":"local","repo":"org/checkpoints"}}}` + require.NoError(t, os.MkdirAll(filepath.Join(deviceA, ".entire"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(deviceA, ".entire", "settings.json"), []byte(settingsBody), 0o644)) + t.Chdir(deviceA) + + repoA, err := git.PlainOpen(deviceA) + require.NoError(t, err) + stores, err := checkpoint.Open(context.Background(), repoA, checkpoint.OpenOptions{}) + require.NoError(t, err) + + cid := id.CheckpointID("01KVBJCWYA4YW6J5M9GP655HZN") + const sessionID = "session-from-device-a" + require.NoError(t, stores.Persistent.Write(context.Background(), checkpoint.Session{ + CheckpointID: cid, + SessionID: sessionID, + Strategy: "manual-commit", + Transcript: redact.AlreadyRedacted([]byte("transcript from A")), + Prompts: []string{"do the thing on device A"}, + FilesTouched: []string{"a.go"}, + AuthorName: "Test", + AuthorEmail: "test@example.com", + })) + + refName, err := checkpoint.RefName(cid) + require.NoError(t, err) + gitRun(t, deviceA, "push", "-q", "origin", refName.String()+":"+refName.String()) + + testutil.WriteFile(t, deviceA, "feature.txt", "from A") + testutil.GitAdd(t, deviceA, "feature.txt") + msgPath := filepath.Join(deviceA, ".git", "COMMIT_EDITMSG_TEST") + require.NoError(t, os.WriteFile(msgPath, []byte(trailers.FormatCheckpoint("feat from device A", cid)), 0o644)) + gitRun(t, deviceA, "commit", "-F", msgPath) + gitRun(t, deviceA, "push", "-q", "origin", "HEAD:"+branch) + + deviceB := filepath.Join(t.TempDir(), "device-b") + gitRun(t, t.TempDir(), "clone", "-q", "--branch", branch, bareURL, deviceB) + require.NoError(t, os.MkdirAll(filepath.Join(deviceB, ".entire"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(deviceB, ".entire", "settings.json"), []byte(settingsBody), 0o644)) + + // Clone must not have brought the checkpoint ref — that is the second-device gap. + verify := exec.CommandContext(context.Background(), "git", "show-ref", "--verify", "--quiet", refName.String()) + verify.Dir = deviceB + verify.Env = testutil.GitIsolatedEnv() + require.Error(t, verify.Run(), "device B must lack the checkpoint ref locally before discovery") + + logMsg := gitOutput(t, deviceB, "log", "-1", "--format=%B") + require.Contains(t, logMsg, cid.String(), "device B HEAD must carry the Entire-Checkpoint trailer") + + t.Chdir(deviceB) + repoB, err := git.PlainOpen(deviceB) + require.NoError(t, err) + + points, _, err := getBranchCheckpoints(context.Background(), repoB, 10) + require.NoError(t, err) + require.NotEmpty(t, points, "discovered+hydrated checkpoint must appear on device B") + + var found bool + for _, p := range points { + if p.CheckpointID == cid { + found = true + assert.Equal(t, sessionID, p.SessionID, "hydration must fill SessionID for --session filters") + assert.Equal(t, 1, p.SessionCount) + assert.Contains(t, p.SessionIDs, sessionID) + assert.False(t, p.Date.IsZero()) + break + } + } + require.True(t, found, "RewindPoint for remote-discovered checkpoint %s missing; got %+v", cid, points) +} diff --git a/cmd/entire/cli/git_operations.go b/cmd/entire/cli/git_operations.go index 3cc4c7fb06..375b4864d1 100644 --- a/cmd/entire/cli/git_operations.go +++ b/cmd/entire/cli/git_operations.go @@ -510,11 +510,12 @@ const checkpointRefListTimeout = 5 * time.Second // on another machine that have no local ref yet, then hydrates each lazily on // read through FetchCheckpointRef. // -// Enumeration is checkpoint-remote-scoped, matching the on-demand fetch and PR -// #1719's authority rule: with a checkpoint_remote configured it queries that -// remote, and with none configured it returns (nil, nil) so List stays -// local-only rather than scanning origin. This keeps the default (no -// checkpoint_remote) behavior unchanged. +// Scope (deliberately stricter than the on-demand read fetch): +// - no checkpoint_remote configured → (nil, nil), List stays local-only +// (unlike FetchCheckpointRef / remote.FetchURL, which fall back to origin); +// - with checkpoint_remote configured → queries the resolved checkpoint URL +// via remote.FetchURL (which can still fall through to origin in edge cases +// such as settings-load failure or an underivable checkpoint URL). // // Resolution and ls-remote are pinned to the worktree root (not process cwd) so // repo-local git config (url.*.insteadOf, credential helpers, remotes) applies. @@ -545,8 +546,11 @@ func ListCheckpointRefsOnRemote(ctx context.Context) ([]plumbing.ReferenceName, // parseCheckpointRefNames extracts the checkpoint ref names from `git ls-remote` // output. Each line is "\t"; only refs under CheckpointRefPrefix -// are kept (the store re-validates each via ParseRef). Peeled tag lines -// ("\t^{}") never carry the checkpoint prefix, so they drop out. +// are kept (the store re-validates each via ParseRef). Checkpoint refs point at +// commits so no peeled (`^{}`) lines appear for them; refs/tags peeled lines +// lack the checkpoint prefix and drop out here; any anomalous +// refs/entire/checkpoints/...^{} name is rejected by ParseRef downstream (the +// "{}" shard never matches ShardFor). func parseCheckpointRefNames(output []byte) []plumbing.ReferenceName { var names []plumbing.ReferenceName for _, line := range strings.Split(string(output), "\n") { diff --git a/cmd/entire/cli/git_operations_test.go b/cmd/entire/cli/git_operations_test.go index c100f423bd..26665a5d5c 100644 --- a/cmd/entire/cli/git_operations_test.go +++ b/cmd/entire/cli/git_operations_test.go @@ -7,7 +7,6 @@ import ( "path/filepath" "strings" "testing" - "time" "github.com/entireio/cli/cmd/entire/cli/checkpoint" "github.com/entireio/cli/cmd/entire/cli/checkpoint/remote" @@ -703,8 +702,7 @@ func TestParseCheckpointRefNames_RealLsRemote(t *testing.T) { // TestListCheckpointRefsOnRemote_NotConfigured proves the authority gate: with // no checkpoint_remote configured, enumeration is a no-op (nil, no error, no -// network) so List behavior stays unchanged (local-only) — never scanning -// origin. Not parallel: uses t.Chdir. +// network) so List stays local-only. Not parallel: uses t.Chdir. func TestListCheckpointRefsOnRemote_NotConfigured(t *testing.T) { dir := t.TempDir() testutil.InitRepo(t, dir) @@ -718,16 +716,35 @@ func TestListCheckpointRefsOnRemote_NotConfigured(t *testing.T) { assert.Nil(t, names, "no checkpoint_remote configured must leave List local-only (no remote enumeration)") } -// TestListCheckpointRefsOnRemote_ResolvesFromSubdir proves worktree pinning: -// enumeration resolves the worktree root (and therefore repo-local settings) -// even when the process cwd is a subdirectory, rather than depending on cwd -// alone. Not parallel: uses t.Chdir. +// TestListCheckpointRefsOnRemote_ResolvesFromSubdir proves worktree pinning for +// the configured path: settings + ls-remote run from the worktree root even when +// process cwd is a subdirectory. Uses a local bare remote and an unknown +// checkpoint_remote provider so FetchURL falls back to origin (offline). +// Not parallel: uses t.Chdir. func TestListCheckpointRefsOnRemote_ResolvesFromSubdir(t *testing.T) { + bareDir := t.TempDir() + gitRun(t, bareDir, "init", "--bare", "-q", bareDir) + dir := t.TempDir() testutil.InitRepo(t, dir) testutil.WriteFile(t, dir, "f.txt", "init") testutil.GitAdd(t, dir, "f.txt") testutil.GitCommit(t, dir, "init") + head := gitOutput(t, dir, "rev-parse", "HEAD") + ref := "refs/entire/checkpoints/ZN/01KVBJCWYA4YW6J5M9GP655HZN" + gitRun(t, dir, "update-ref", ref, head) + gitRun(t, dir, "remote", "add", "origin", bareDir) + gitRun(t, dir, "push", "-q", "origin", ref+":"+ref) + + entireDir := filepath.Join(dir, ".entire") + require.NoError(t, os.MkdirAll(entireDir, 0o755)) + // provider "local" is unknown to providerHost → FetchURL falls back to origin + // (the bare path) after file:// derivation fails — keeps this offline. + require.NoError(t, os.WriteFile( + filepath.Join(entireDir, "settings.json"), + []byte(`{"enabled":true,"strategy_options":{"checkpoint_remote":{"provider":"local","repo":"org/checkpoints"}}}`), + 0o644, + )) sub := filepath.Join(dir, "nested", "deep") require.NoError(t, os.MkdirAll(sub, 0o755)) @@ -735,14 +752,37 @@ func TestListCheckpointRefsOnRemote_ResolvesFromSubdir(t *testing.T) { names, err := ListCheckpointRefsOnRemote(context.Background()) require.NoError(t, err) - assert.Nil(t, names, "no checkpoint_remote from a subdir must still be a no-op (worktree-resolved settings)") + require.Len(t, names, 1) + assert.Equal(t, ref, names[0].String()) } -// TestCheckpointRefListTimeoutIsShort pins the discovery budget: user-facing -// list/explain must not inherit a long fetch-style hang when the remote is -// unreachable (best-effort fallback to local refs). -func TestCheckpointRefListTimeoutIsShort(t *testing.T) { - t.Parallel() - assert.LessOrEqual(t, checkpointRefListTimeout, 5*time.Second) - assert.Greater(t, checkpointRefListTimeout, time.Duration(0)) +// TestListCheckpointRefsOnRemote_HonorsCanceledContext proves the discovery +// timeout context reaches the git subprocess: an already-canceled ctx with a +// configured checkpoint_remote must error (not hang / not silently succeed). +// Not parallel: uses t.Chdir. +func TestListCheckpointRefsOnRemote_HonorsCanceledContext(t *testing.T) { + bareDir := t.TempDir() + gitRun(t, bareDir, "init", "--bare", "-q", bareDir) + + dir := t.TempDir() + testutil.InitRepo(t, dir) + testutil.WriteFile(t, dir, "f.txt", "init") + testutil.GitAdd(t, dir, "f.txt") + testutil.GitCommit(t, dir, "init") + gitRun(t, dir, "remote", "add", "origin", bareDir) + + entireDir := filepath.Join(dir, ".entire") + require.NoError(t, os.MkdirAll(entireDir, 0o755)) + require.NoError(t, os.WriteFile( + filepath.Join(entireDir, "settings.json"), + []byte(`{"enabled":true,"strategy_options":{"checkpoint_remote":{"provider":"local","repo":"org/checkpoints"}}}`), + 0o644, + )) + t.Chdir(dir) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + _, err := ListCheckpointRefsOnRemote(ctx) + require.Error(t, err, "canceled context must reach ls-remote (regression: deleting WithTimeout would still pass a constant-only test)") } diff --git a/docs/architecture/ref-checkpoint-backend.md b/docs/architecture/ref-checkpoint-backend.md index ab77eb9386..a7a1eaac25 100644 --- a/docs/architecture/ref-checkpoint-backend.md +++ b/docs/architecture/ref-checkpoint-backend.md @@ -126,9 +126,11 @@ To close that gap `List` supports **opt-in remote discovery**: - The caller marks the context with `WithRemoteListDiscovery` (set only on explicit, user-facing enumeration — `entire checkpoint list` and the branch `explain` view — never the per-turn commit hook, so routine local listings stay network-free). - A **remote ref lister** injected by the CLI runs a single `git ls-remote refs/entire/checkpoints/*` — **names only, no object transfer** — against the checkpoint remote. -- Each advertised ref that has no local ref yet is added to the result as a not-yet-hydrated `CheckpointInfo`. Its `CreatedAt` is recovered from the ULID timestamp in the ref name, so it sorts by real recency without a fetch; the rest of its contents are **hydrated lazily on the next read** via the on-demand ref fetch. +- Each advertised ref that has no local ref yet is added to the result as a not-yet-hydrated `CheckpointInfo`. Its `CreatedAt` is recovered from the ULID timestamp in the ref name, so it sorts by real recency without a fetch; the rest of its contents are **hydrated lazily on the next read** via the on-demand ref fetch (and, on the branch `explain` / `checkpoint list` path, eagerly for the truncated display set so `--session` filters see real SessionIDs). -Enumeration is **checkpoint-remote-scoped** — the same authority rule as the on-demand fetch and #1719: with a `checkpoint_remote` configured it queries that remote, and with **none** configured it does nothing (returns no refs), leaving `List` local-only rather than scanning origin. Discovery is also **best-effort and additive**: an `ls-remote` failure or a short timeout (a few seconds — this path must not turn a previously-local listing into a long hang when the remote is unreachable) logs and returns the local results rather than failing the whole listing. URL resolution and `ls-remote` run from the worktree root so repo-local git config applies. +**Surfacing vs discovery:** storage-level `List` can return remote-only stubs, but the branch view (`getBranchCheckpoints`) only surfaces a discovered checkpoint when an `Entire-Checkpoint` trailer for that ID is present in the scanned local commit range. Discovery therefore helps only after the user has pulled the branch commits that carry those trailers — it does not invent branch entries from remote refs alone. + +Enumeration is **checkpoint-remote-scoped** and **stricter than the on-demand read fetch**: with a `checkpoint_remote` configured it queries the resolved checkpoint URL (`remote.FetchURL`, which can fall through to origin in edge cases); with **none** configured the lister returns no refs and `List` stays local-only — unlike by-ID reads, which fall back to origin when nothing is configured. Discovery is also **best-effort and additive**: an `ls-remote` failure or a short timeout (a few seconds — this path must not turn a previously-local listing into a long hang when the remote is unreachable) logs, warns on stderr, and returns the local results rather than failing the whole listing. URL resolution and `ls-remote` run from the worktree root so repo-local git config applies. ## Read routing and coexistence From 577920da417b88d58a71f078c68bfc2ebaed41f3 Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Tue, 21 Jul 2026 20:13:42 -0400 Subject: [PATCH 5/6] fix(checkpoint): cap List stub hydration pass duration Bound the whole remote-stub hydration loop with ListHydrationPassTimeout so a slow remote cannot burn stub_count * ListHydrationTimeout. Co-authored-by: Cursor --- cmd/entire/cli/checkpoint/refs_store.go | 5 +++++ cmd/entire/cli/explain.go | 24 +++++++++++++++++------- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/cmd/entire/cli/checkpoint/refs_store.go b/cmd/entire/cli/checkpoint/refs_store.go index 76a497f133..f49c0ae63e 100644 --- a/cmd/entire/cli/checkpoint/refs_store.go +++ b/cmd/entire/cli/checkpoint/refs_store.go @@ -24,6 +24,11 @@ import ( // into many minutes of sequential ref fetches. const ListHydrationTimeout = 15 * time.Second +// ListHydrationPassTimeout bounds the entire List/explain stub-hydration pass. +// Without this, a slow remote can burn stub_count * ListHydrationTimeout +// (limit defaults to 100 and is user-settable via --limit). +const ListHydrationPassTimeout = 30 * time.Second + var ( _ PersistentStore = (*gitRefsStore)(nil) _ AuthorReader = (*gitRefsStore)(nil) diff --git a/cmd/entire/cli/explain.go b/cmd/entire/cli/explain.go index 2eb3ff6f80..3171a696af 100644 --- a/cmd/entire/cli/explain.go +++ b/cmd/entire/cli/explain.go @@ -2248,11 +2248,12 @@ func getBranchCheckpoints(ctx context.Context, repo *git.Repository, limit int) } // hydrateListedBranchCheckpoints fills SessionID/etc for remote-discovered List -// stubs among the already-truncated RewindPoints. Per-ref fetch budget is -// ListHydrationTimeout (much shorter than the default on-demand fetch). Failures -// clear ListedStub (fail-once) inside HydrateListedCheckpointInfo; when any -// stub still lacks SessionID afterward we note it on stderr so --session -// filters dropping them is not silent. +// stubs among the already-truncated RewindPoints. The whole pass is capped by +// ListHydrationPassTimeout, and each ref additionally gets ListHydrationTimeout +// (much shorter than the default on-demand fetch). Failures clear ListedStub +// (fail-once) inside HydrateListedCheckpointInfo; when any stub still lacks +// SessionID afterward we note it on stderr so --session filters dropping them +// is not silent. func hydrateListedBranchCheckpoints( ctx context.Context, store interface { @@ -2263,6 +2264,9 @@ func hydrateListedBranchCheckpoints( points []strategy.RewindPoint, committedByID map[id.CheckpointID]checkpoint.CheckpointInfo, ) { + passCtx, passCancel := context.WithTimeout(ctx, checkpoint.ListHydrationPassTimeout) + defer passCancel() + hydrationFailed := 0 for i := range points { cpID := points[i].CheckpointID @@ -2273,7 +2277,13 @@ func hydrateListedBranchCheckpoints( if !ok || !cpInfo.ListedStub { continue } - hctx, cancel := context.WithTimeout(ctx, checkpoint.ListHydrationTimeout) + if err := passCtx.Err(); err != nil { + // Overall budget exhausted: leave remaining stubs unhydrated and + // count them toward the user-facing warning. + hydrationFailed++ + continue + } + hctx, cancel := context.WithTimeout(passCtx, checkpoint.ListHydrationTimeout) hydrated := checkpoint.HydrateListedCheckpointInfo(hctx, store, cpInfo) cancel() committedByID[cpID] = hydrated @@ -2284,7 +2294,7 @@ func hydrateListedBranchCheckpoints( points[i].ToolUseID = hydrated.ToolUseID points[i].Agent = hydrated.Agent if hydrated.SessionCount > 0 { - points[i].SessionPrompt = readLatestCommittedSessionPrompt(ctx, store, cpID, hydrated.SessionCount) + points[i].SessionPrompt = readLatestCommittedSessionPrompt(passCtx, store, cpID, hydrated.SessionCount) } if hydrated.SessionID == "" { hydrationFailed++ From 93ab0d502803b85965f39281f26cdf066bee9aaf Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Tue, 21 Jul 2026 20:24:53 -0400 Subject: [PATCH 6/6] fix(remote): redact credentialed URLs in ls-remote errors Co-authored-by: Cursor --- cmd/entire/cli/checkpoint/remote/git.go | 9 +++++++-- cmd/entire/cli/checkpoint/remote/git_test.go | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/cmd/entire/cli/checkpoint/remote/git.go b/cmd/entire/cli/checkpoint/remote/git.go index be9795ad91..b80534a4ef 100644 --- a/cmd/entire/cli/checkpoint/remote/git.go +++ b/cmd/entire/cli/checkpoint/remote/git.go @@ -462,7 +462,7 @@ func lsRemote(ctx context.Context, dir, remote string, patterns ...string) ([]by disableTerminalPrompt(cmd) out, err := cmd.Output() if err != nil { - return out, fmt.Errorf("git ls-remote: %w", formatGitCommandError(ctx, err)) + return out, fmt.Errorf("git ls-remote: %w", formatGitCommandError(ctx, err, remote)) } return out, nil } @@ -470,7 +470,9 @@ func lsRemote(ctx context.Context, dir, remote string, patterns ...string) ([]by // formatGitCommandError enriches an exec error from git Output() so callers see // useful detail: context deadline expiry by name, and git's stderr (auth denied, // repository not found, DNS) which ExitError otherwise hides behind "exit status N". -func formatGitCommandError(ctx context.Context, err error) error { +// When remote is a URL it may carry credentials that git echoes into stderr; +// those are redacted before the error is returned (same pattern as FetchBlobs). +func formatGitCommandError(ctx context.Context, err error, remote string) error { if err == nil { return nil } @@ -480,6 +482,9 @@ func formatGitCommandError(ctx context.Context, err error) error { var exitErr *exec.ExitError if errors.As(err, &exitErr) { if stderr := strings.TrimSpace(string(exitErr.Stderr)); stderr != "" { + if remote != "" { + stderr = strings.ReplaceAll(stderr, remote, RedactURL(remote)) + } // Collapse whitespace so multi-line git stderr stays one log/attr value. stderr = strings.Join(strings.Fields(stderr), " ") return fmt.Errorf("%w (%s)", err, stderr) diff --git a/cmd/entire/cli/checkpoint/remote/git_test.go b/cmd/entire/cli/checkpoint/remote/git_test.go index 9a274dcaae..25909830fe 100644 --- a/cmd/entire/cli/checkpoint/remote/git_test.go +++ b/cmd/entire/cli/checkpoint/remote/git_test.go @@ -1274,3 +1274,21 @@ func TestIsNonInteractiveSSH(t *testing.T) { assert.False(t, IsNonInteractiveSSH(context.Background())) assert.True(t, IsNonInteractiveSSH(WithNonInteractiveSSH(context.Background()))) } + +func TestFormatGitCommandError_RedactsRemoteURL(t *testing.T) { + t.Parallel() + + remote := "https://user:hunter2@github.com/org/repo.git" + // Output() populates ExitError.Stderr (Run() does not). + cmd := exec.CommandContext(context.Background(), "sh", "-c", + fmt.Sprintf(`printf 'fatal: repository "%s" not found\n' >&2; exit 128`, remote)) + _, err := cmd.Output() + require.Error(t, err) + + formatted := formatGitCommandError(context.Background(), err, remote) + require.Error(t, formatted) + msg := formatted.Error() + assert.NotContains(t, msg, "hunter2") + assert.NotContains(t, msg, "user:hunter2") + assert.Contains(t, msg, RedactURL(remote)) +}