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 891ae16586..a24fa33410 100644 --- a/cmd/entire/cli/checkpoint/fetching_tree.go +++ b/cmd/entire/cli/checkpoint/fetching_tree.go @@ -23,6 +23,21 @@ 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. +// +// 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. // 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..76632c1153 100644 --- a/cmd/entire/cli/checkpoint/id/id_test.go +++ b/cmd/entire/cli/checkpoint/id/id_test.go @@ -1,13 +1,53 @@ package id import ( + "bytes" "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() + // 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) + } + 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_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 a8849b965d..f49c0ae63e 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,17 @@ 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 + +// 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) @@ -32,8 +45,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 +66,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 +403,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 +426,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 +441,155 @@ 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, 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 { + 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. +// 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, ListedStub: true} + if createdAt, ok := cid.Time(); ok { + info.CreatedAt = createdAt + } + return info +} + +// listedCheckpointNeedsHydration reports whether info is a names-only List stub +// 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.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 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) +}, info CheckpointInfo) CheckpointInfo { + if !listedCheckpointNeedsHydration(info) { + return info + } + 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 + } + + 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 != "" { + out.SessionIDs = append(out.SessionIDs, meta.SessionID) + } + if i == len(summary.Sessions)-1 { + out.Agent = meta.Agent + out.SessionID = meta.SessionID + if !meta.CreatedAt.IsZero() { + out.CreatedAt = meta.CreatedAt + } + out.IsTask = meta.IsTask + out.ToolUseID = meta.ToolUseID + lastMetaOK = true + } + } + 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 // 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..2d68c066ad 100644 --- a/cmd/entire/cli/checkpoint/refs_store_test.go +++ b/cmd/entire/cli/checkpoint/refs_store_test.go @@ -118,6 +118,187 @@ 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) + }) +} + +// 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.True(t, stub.ListedStub) + 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)) + 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: 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")) + 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) { 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/remote/git.go b/cmd/entire/cli/checkpoint/remote/git.go index 1de3ef55fb..b80534a4ef 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,37 @@ 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, remote)) } 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". +// 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 + } + 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 != "" { + 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) + } + } + 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/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)) +} 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 48f1d9a260..3171a696af 100644 --- a/cmd/entire/cli/explain.go +++ b/cmd/entire/cli/explain.go @@ -2087,14 +2087,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 } @@ -2130,6 +2140,11 @@ func getBranchCheckpoints(ctx context.Context, repo *git.Repository, limit int) if !found { return } + // 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{ @@ -2145,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) } @@ -2210,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. @@ -2226,6 +2247,64 @@ 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. 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 { + 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, +) { + passCtx, passCancel := context.WithTimeout(ctx, checkpoint.ListHydrationPassTimeout) + defer passCancel() + + hydrationFailed := 0 + for i := range points { + cpID := points[i].CheckpointID + if cpID.IsEmpty() { + continue + } + cpInfo, ok := committedByID[cpID] + if !ok || !cpInfo.ListedStub { + continue + } + 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 + 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(passCtx, 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 @@ -2619,11 +2698,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_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/explain_test.go b/cmd/entire/cli/explain_test.go index 10b94cd515..e9486e112f 100644 --- a/cmd/entire/cli/explain_test.go +++ b/cmd/entire/cli/explain_test.go @@ -4271,6 +4271,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) { diff --git a/cmd/entire/cli/git_operations.go b/cmd/entire/cli/git_operations.go index 195103485b..375b4864d1 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" @@ -495,6 +496,77 @@ func FetchCheckpointRef(ctx context.Context, ref plumbing.ReferenceName) error { return nil } +// 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 +// 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. +// +// 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. +func ListCheckpointRefsOnRemote(ctx context.Context) ([]plumbing.ReferenceName, error) { + if !remote.Configured(ctx) { + return nil, nil + } + + 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) + } + + ctx, cancel := context.WithTimeout(ctx, checkpointRefListTimeout) + defer cancel() + + 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) + } + 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). 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") { + 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..26665a5d5c 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,152 @@ 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 stays local-only. 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)") +} + +// 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)) + t.Chdir(sub) + + names, err := ListCheckpointRefsOnRemote(context.Background()) + require.NoError(t, err) + require.Len(t, names, 1) + assert.Equal(t, ref, names[0].String()) +} + +// 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 31f4988113..a7a1eaac25 100644 --- a/docs/architecture/ref-checkpoint-backend.md +++ b/docs/architecture/ref-checkpoint-backend.md @@ -118,7 +118,19 @@ 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 (and, on the branch `explain` / `checkpoint list` path, eagerly for the truncated display set so `--session` filters see real SessionIDs). + +**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 @@ -211,6 +223,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.