From 3848cac26f3a8e288284ad36be5e8467eef5d956 Mon Sep 17 00:00:00 2001 From: Peyton Montei Date: Mon, 20 Jul 2026 13:00:52 -0700 Subject: [PATCH 1/3] fix(checkpoint): kind-route backfill writes to the store holding the checkpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backfill writes (SessionSummary, SessionTranscript, CheckpointAttribution) always went to the configured primary, while reads are kind-routed across both git backends. Under a git-refs primary, a pre-migration hex checkpoint that only exists on the v1 branch resolved fine on read but failed every backfill with ErrCheckpointNotFound — 'entire checkpoint explain --generate' generated the AI summary, then discarded it on save. Backfills now follow the same store order as reads, falling through on ErrCheckpointNotFound. A backfill landing on the primary still fans out to mirrors; one landing on a fallback store skips them (mirrors follow the primary). Creates (Session) are unchanged: their ID is minted to match the primary's format. Co-Authored-By: Claude Fable 5 --- cmd/entire/cli/checkpoint/open.go | 9 +- cmd/entire/cli/checkpoint/routing_store.go | 76 +++++++++- .../cli/checkpoint/routing_store_test.go | 136 ++++++++++++++++++ 3 files changed, 211 insertions(+), 10 deletions(-) diff --git a/cmd/entire/cli/checkpoint/open.go b/cmd/entire/cli/checkpoint/open.go index 13e954b922..996b499c2a 100644 --- a/cmd/entire/cli/checkpoint/open.go +++ b/cmd/entire/cli/checkpoint/open.go @@ -80,10 +80,11 @@ func Open(ctx context.Context, repo *git.Repository, opts OpenOptions) (*Stores, } writer := newFanoutStore(primary, mirrors) - // Read routing: resolve id-keyed reads by the checkpoint's format across both - // git backends (a ULID lives in refs, a hex ID on the branch or a migrated - // ref), so a coexisting / mid-migration repo reads either format without - // reconfiguring. Writes still go through writer (configured primary + mirrors). + // Kind routing: resolve id-keyed reads and backfill writes by the + // checkpoint's format across both git backends (a ULID lives in refs, a hex + // ID on the branch or a migrated ref), so a coexisting / mid-migration repo + // handles either format without reconfiguring. Creates still go through + // writer (configured primary + mirrors). branchStore, refsStore, err := buildKindReadStores(ctx, env, primaryType, primary) if err != nil { return nil, err diff --git a/cmd/entire/cli/checkpoint/routing_store.go b/cmd/entire/cli/checkpoint/routing_store.go index 123e453856..3f06654efb 100644 --- a/cmd/entire/cli/checkpoint/routing_store.go +++ b/cmd/entire/cli/checkpoint/routing_store.go @@ -20,10 +20,11 @@ import ( // a git-branch primary the branch is authoritative for hex, so refs is not // consulted. // -// List unions both backends (disjoint ID spaces). Writes are NOT kind-routed: -// they go to the configured primary (+ mirrors) via writer, since a new -// checkpoint's ID is already minted to match the primary's format -// (see checkpoint.GenerateCheckpointID). +// List unions both backends (disjoint ID spaces). Creates (Session) are NOT +// kind-routed: they go to the configured primary (+ mirrors) via writer, since +// a new checkpoint's ID is already minted to match the primary's format (see +// checkpoint.GenerateCheckpointID). Backfills update an existing checkpoint, +// so they follow the same store order as reads — see Write. type kindRoutingStore struct { writer PersistentStore // configured primary + mirrors (fanout); handles Write branch PersistentStore // git-branch store; serves hex reads @@ -183,9 +184,72 @@ func (s *kindRoutingStore) ReadSessionMetadataAndPrompts(ctx context.Context, ch return mp.meta, mp.prompts, err } -// Write is not kind-routed: it targets the configured primary (+ mirrors). +// Write routes a create (Session) to the configured primary (+ mirrors): a new +// checkpoint's ID is already minted to match the primary's format (see +// checkpoint.GenerateCheckpointID). Backfills target an EXISTING checkpoint, +// which — like reads — may live in either git backend (e.g. a pre-migration hex +// checkpoint still on the v1 branch under a git-refs primary), so they follow +// the read order, falling through to the next store on ErrCheckpointNotFound. func (s *kindRoutingStore) Write(ctx context.Context, req WriteRequest) error { - return s.writer.Write(ctx, req) //nolint:wrapcheck // primary error is the operation's error, surfaced verbatim + checkpointID, isBackfill := backfillTarget(req) + if !isBackfill { + return s.writer.Write(ctx, req) //nolint:wrapcheck // primary error is the operation's error, surfaced verbatim + } + stores := s.backfillOrder(checkpointID) + var err error + for _, st := range stores { + err = st.Write(ctx, req) + if !errors.Is(err, ErrCheckpointNotFound) { + return err //nolint:wrapcheck // in-package store error surfaced verbatim + } + } + return err //nolint:wrapcheck // ErrCheckpointNotFound from the final store, surfaced verbatim +} + +// backfillTarget returns the checkpoint ID a backfill request updates. +// ok is false for Session (a create) and unknown request types, which are not +// kind-routed. +func backfillTarget(req WriteRequest) (id.CheckpointID, bool) { + switch r := req.(type) { + case SessionTranscript: + return r.CheckpointID, true + case SessionSummary: + return r.CheckpointID, true + case CheckpointAttribution: + return r.CheckpointID, true + default: + return id.EmptyCheckpointID, false + } +} + +// backfillOrder returns the write targets for a backfill of checkpointID, in +// the same priority order reads use. The store that is the configured primary +// is replaced by writer, so a backfill landing on the primary still fans out to +// mirrors; a backfill landing on a fallback store deliberately skips mirrors +// (mirrors follow the primary). +func (s *kindRoutingStore) backfillOrder(checkpointID id.CheckpointID) []PersistentStore { + order := s.readOrder(checkpointID) + targets := make([]PersistentStore, len(order)) + for i, st := range order { + if s.isPrimary(st) { + targets[i] = s.writer + } else { + targets[i] = st + } + } + return targets +} + +// isPrimary reports whether st is the configured primary's read store. +func (s *kindRoutingStore) isPrimary(st PersistentStore) bool { + switch s.primaryType { + case BackendTypeGitBranch: + return st == s.branch + case BackendTypeGitRefs: + return st == s.refs + default: + return false + } } // kindRoutingStoreWithAuthor adds the optional AuthorReader capability, routing diff --git a/cmd/entire/cli/checkpoint/routing_store_test.go b/cmd/entire/cli/checkpoint/routing_store_test.go index 21760ab8f5..c9e4f29e0c 100644 --- a/cmd/entire/cli/checkpoint/routing_store_test.go +++ b/cmd/entire/cli/checkpoint/routing_store_test.go @@ -194,6 +194,142 @@ func TestKindRoutingStore_ListDedupesAcrossBackends(t *testing.T) { assert.Equal(t, 1, count, "a checkpoint present in both backends should appear once") } +func TestKindRoutingStore_SummaryBackfillFallsBackToBranch(t *testing.T) { + t.Parallel() + ctx := context.Background() + + // Regression: `entire checkpoint explain --generate ` under a + // git-refs primary. The checkpoint resolves via the branch read fallback, + // but the summary write went only to the refs primary, which has no ref + // for it, so the generated summary was discarded with ErrCheckpointNotFound. + hexID := id.MustCheckpointID("a1b2c3d4e5f6") + _, repo, _ := newTestRepo(t) + branch := NewGitStore(repo, DefaultV1Refs()) + refs := newGitRefsStore(repo) + writeRoutingCheckpoint(t, branch, hexID, "hex-on-branch") + + router := newKindRoutingStore(refs, branch, refs, BackendTypeGitRefs) + + err := router.Write(ctx, SessionSummary{ + CheckpointID: hexID, + Summary: &Summary{Intent: "test intent", Outcome: "test outcome"}, + }) + require.NoError(t, err, "summary backfill for a pre-migration hex checkpoint must fall back to the branch store") + + meta, err := router.ReadSessionMetadata(ctx, hexID, 0) + require.NoError(t, err) + require.NotNil(t, meta.Summary, "backfilled summary should be readable back through the router") + assert.Equal(t, "test intent", meta.Summary.Intent) +} + +func TestKindRoutingStore_TranscriptAndAttributionBackfillsFallBackToBranch(t *testing.T) { + t.Parallel() + ctx := context.Background() + + hexID := id.MustCheckpointID("a1b2c3d4e5f6") + _, repo, _ := newTestRepo(t) + branch := NewGitStore(repo, DefaultV1Refs()) + refs := newGitRefsStore(repo) + writeRoutingCheckpoint(t, branch, hexID, "hex-on-branch") + + router := newKindRoutingStore(refs, branch, refs, BackendTypeGitRefs) + + err := router.Write(ctx, SessionTranscript{ + CheckpointID: hexID, + SessionID: "hex-on-branch", + Transcript: redact.AlreadyRedacted([]byte("finalized transcript")), + }) + require.NoError(t, err, "transcript backfill for a hex checkpoint on the branch must fall back to the branch store") + + err = router.Write(ctx, CheckpointAttribution{ + CheckpointID: hexID, + Attribution: &Attribution{AgentLines: 7}, + }) + require.NoError(t, err, "attribution backfill for a hex checkpoint on the branch must fall back to the branch store") +} + +func TestKindRoutingStore_BackfillULIDRoutesToRefs(t *testing.T) { + t.Parallel() + ctx := context.Background() + + // A ULID checkpoint only ever lives in refs, so its backfill must reach the + // refs store even when the configured primary is git-branch. + ulidID := id.MustCheckpointID(routingSampleULID) + _, repo, _ := newTestRepo(t) + branch := NewGitStore(repo, DefaultV1Refs()) + refs := newGitRefsStore(repo) + writeRoutingCheckpoint(t, refs, ulidID, "ulid-in-refs") + + router := newKindRoutingStore(branch, branch, refs, BackendTypeGitBranch) + + err := router.Write(ctx, SessionSummary{ + CheckpointID: ulidID, + Summary: &Summary{Intent: "ulid intent"}, + }) + require.NoError(t, err, "summary backfill for a ULID checkpoint must route to refs under a git-branch primary") + + meta, err := router.ReadSessionMetadata(ctx, ulidID, 0) + require.NoError(t, err) + require.NotNil(t, meta.Summary) + assert.Equal(t, "ulid intent", meta.Summary.Intent) +} + +func TestKindRoutingStore_BackfillMissingEverywhereReturnsNotFound(t *testing.T) { + t.Parallel() + ctx := context.Background() + + _, repo, _ := newTestRepo(t) + branch := NewGitStore(repo, DefaultV1Refs()) + refs := newGitRefsStore(repo) + + router := newKindRoutingStore(refs, branch, refs, BackendTypeGitRefs) + + err := router.Write(ctx, SessionSummary{ + CheckpointID: id.MustCheckpointID("a1b2c3d4e5f6"), + Summary: &Summary{Intent: "orphan"}, + }) + require.ErrorIs(t, err, ErrCheckpointNotFound, "a backfill for a checkpoint absent from every backend still reports not-found") +} + +func TestKindRoutingStore_BackfillMirrorFanout(t *testing.T) { + t.Parallel() + ctx := context.Background() + + hexID := id.MustCheckpointID("a1b2c3d4e5f6") + + t.Run("backfill landing on the primary still fans out to mirrors", func(t *testing.T) { + t.Parallel() + _, repo, _ := newTestRepo(t) + branch := NewGitStore(repo, DefaultV1Refs()) + refs := newGitRefsStore(repo) + writeRoutingCheckpoint(t, refs, hexID, "hex-migrated-to-refs") + + mirror := &fakeMirror{} + writer := newFanoutStore(refs, []Writer{mirror}) + router := newKindRoutingStore(writer, branch, refs, BackendTypeGitRefs) + + req := SessionSummary{CheckpointID: hexID, Summary: &Summary{Intent: "mirrored"}} + require.NoError(t, router.Write(ctx, req)) + assert.Len(t, mirror.writes, 1, "a backfill served by the primary should reach mirrors") + }) + + t.Run("backfill falling back to the branch skips mirrors", func(t *testing.T) { + t.Parallel() + _, repo, _ := newTestRepo(t) + branch := NewGitStore(repo, DefaultV1Refs()) + refs := newGitRefsStore(repo) + writeRoutingCheckpoint(t, branch, hexID, "hex-on-branch") + + mirror := &fakeMirror{} + writer := newFanoutStore(refs, []Writer{mirror}) + router := newKindRoutingStore(writer, branch, refs, BackendTypeGitRefs) + + req := SessionSummary{CheckpointID: hexID, Summary: &Summary{Intent: "not mirrored"}} + require.NoError(t, router.Write(ctx, req)) + assert.Empty(t, mirror.writes, "a backfill served by a fallback store must not fan out to mirrors (mirrors follow the primary)") + }) +} + func TestKindRoutingStore_GetCheckpointAuthorRoutes(t *testing.T) { t.Parallel() ctx := context.Background() From 6b961f761bbad13973aa6cd6989572179cbcfd71 Mon Sep 17 00:00:00 2001 From: Peyton Montei Date: Mon, 20 Jul 2026 15:01:05 -0700 Subject: [PATCH 2/3] fix(checkpoint): harden backfill routing per review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings from the PR #1811 multi-agent pass: - Log the fallthrough: a backfill served by a fallback store now emits logging.Info (absent from primary, mirrors skipped) and each skip emits logging.Debug — previously the most consequential routing decision in the store recorded nothing. - Branch-store backfills no longer create the v1 branch as a probe side effect: requireSessionsBranch reports ErrCheckpointNotFound when the sessions branch is absent, instead of ensureSessionsBranch materializing an orphan branch in refs-only repos on a miss. Creates still ensure. - Pin two behaviors with tests: a hard (non-not-found) primary error aborts the fallthrough and leaves the fallback store untouched; a migrated hex checkpoint present in both backends backfills to refs, where refs-first reads can see it. - Document the deliberate write/read fallthrough asymmetry, the local-only backfill absence probe, the closed-union trap in backfillTarget, and the isPrimary default arm. - Update ref-checkpoint-backend.md and sessions-and-checkpoints.md, which still described writes as never kind-routed. Co-Authored-By: Claude Fable 5 --- cmd/entire/cli/checkpoint/persistent.go | 34 ++++++-- cmd/entire/cli/checkpoint/routing_store.go | 39 ++++++++- .../cli/checkpoint/routing_store_test.go | 84 +++++++++++++++++++ docs/architecture/ref-checkpoint-backend.md | 9 +- docs/architecture/sessions-and-checkpoints.md | 7 +- 5 files changed, 157 insertions(+), 16 deletions(-) diff --git a/cmd/entire/cli/checkpoint/persistent.go b/cmd/entire/cli/checkpoint/persistent.go index 6d82820c75..99be9e2764 100644 --- a/cmd/entire/cli/checkpoint/persistent.go +++ b/cmd/entire/cli/checkpoint/persistent.go @@ -857,8 +857,9 @@ func (s *GitStore) backfillAttribution(ctx context.Context, checkpointID id.Chec return err //nolint:wrapcheck // Propagating context cancellation } - if err := s.ensureSessionsBranch(ctx); err != nil { - return fmt.Errorf("failed to ensure sessions branch: %w", err) + // Backfills require the branch to exist; a miss must not create it. + if err := s.requireSessionsBranch(); err != nil { + return err } parentHash, rootTreeHash, err := s.getSessionsBranchRef() @@ -1692,9 +1693,9 @@ func (s *GitStore) backfillSummary(ctx context.Context, checkpointID id.Checkpoi return err //nolint:wrapcheck // Propagating context cancellation } - // Ensure sessions branch exists - if err := s.ensureSessionsBranch(ctx); err != nil { - return fmt.Errorf("failed to ensure sessions branch: %w", err) + // Backfills require the branch to exist; a miss must not create it. + if err := s.requireSessionsBranch(); err != nil { + return err } // Get branch ref and root tree hash (O(1), no flatten) @@ -1740,9 +1741,9 @@ func (s *GitStore) backfillTranscript(ctx context.Context, opts UpdateOptions) e return errors.New("invalid update options: checkpoint ID is required") } - // Ensure sessions branch exists - if err := s.ensureSessionsBranch(ctx); err != nil { - return fmt.Errorf("failed to ensure sessions branch: %w", err) + // Backfills require the branch to exist; a miss must not create it. + if err := s.requireSessionsBranch(); err != nil { + return err } // Get branch ref and root tree hash (O(1), no flatten) @@ -2105,6 +2106,23 @@ func PrecomputeTranscriptBlobs(ctx context.Context, repo *git.Repository, transc }, nil } +// requireSessionsBranch reports ErrCheckpointNotFound when the primary +// metadata ref does not exist. Backfills use this instead of +// ensureSessionsBranch: they target an existing checkpoint, and a missing +// branch trivially implies the checkpoint is absent — creating an orphan +// branch as a side effect of that probe would leave a live v1 branch (List +// union, pre-push) in a repo that never used the git-branch backend. +func (s *GitStore) requireSessionsBranch() error { + _, err := s.repo.Reference(s.refs.Primary, true) + if err == nil { + return nil + } + if errors.Is(err, plumbing.ErrReferenceNotFound) { + return ErrCheckpointNotFound + } + return fmt.Errorf("failed to check sessions branch: %w", err) +} + // ensureSessionsBranch ensures the primary metadata ref exists. func (s *GitStore) ensureSessionsBranch(ctx context.Context) error { _, err := s.repo.Reference(s.refs.Primary, true) diff --git a/cmd/entire/cli/checkpoint/routing_store.go b/cmd/entire/cli/checkpoint/routing_store.go index 3f06654efb..4b36716bba 100644 --- a/cmd/entire/cli/checkpoint/routing_store.go +++ b/cmd/entire/cli/checkpoint/routing_store.go @@ -3,9 +3,12 @@ package checkpoint import ( "context" "errors" + "fmt" + "log/slog" "sort" "github.com/entireio/cli/cmd/entire/cli/checkpoint/id" + "github.com/entireio/cli/cmd/entire/cli/logging" ) // kindRoutingStore resolves id-keyed reads across the two git backends so a repo @@ -24,7 +27,8 @@ import ( // kind-routed: they go to the configured primary (+ mirrors) via writer, since // a new checkpoint's ID is already minted to match the primary's format (see // checkpoint.GenerateCheckpointID). Backfills update an existing checkpoint, -// so they follow the same store order as reads — see Write. +// so they follow the same store order as reads, though only +// ErrCheckpointNotFound falls through (stricter than reads) — see Write. type kindRoutingStore struct { writer PersistentStore // configured primary + mirrors (fanout); handles Write branch PersistentStore // git-branch store; serves hex reads @@ -190,6 +194,14 @@ func (s *kindRoutingStore) ReadSessionMetadataAndPrompts(ctx context.Context, ch // which — like reads — may live in either git backend (e.g. a pre-migration hex // checkpoint still on the v1 branch under a git-refs primary), so they follow // the read order, falling through to the next store on ErrCheckpointNotFound. +// +// The fallthrough is deliberately stricter than read routing's firstResolved +// (which falls through on absent OR any error): only the not-found sentinel +// falls through here. Redirecting a write to another backend after a transient +// primary failure could fork the data, so a hard error aborts and surfaces. +// Note the stores' backfill absence probes are local-only (the refs store's +// refBase does not on-demand fetch like its read path does); a checkpoint +// whose ref exists only remotely backfills to the fallback store. func (s *kindRoutingStore) Write(ctx context.Context, req WriteRequest) error { checkpointID, isBackfill := backfillTarget(req) if !isBackfill { @@ -197,11 +209,27 @@ func (s *kindRoutingStore) Write(ctx context.Context, req WriteRequest) error { } stores := s.backfillOrder(checkpointID) var err error - for _, st := range stores { + for i, st := range stores { err = st.Write(ctx, req) if !errors.Is(err, ErrCheckpointNotFound) { + if err == nil && i > 0 { + // The most consequential routing decision here: the data landed + // somewhere other than the configured primary, and mirrors + // (which follow the primary) were skipped. Record it so "why is + // this backfill on the v1 branch and not in refs / the mirror" + // stays diagnosable. + logging.Info(ctx, "checkpoint: backfill served by fallback store; absent from primary, mirrors skipped", + slog.String("checkpoint_id", checkpointID.String()), + slog.String("request_type", fmt.Sprintf("%T", req))) + } return err //nolint:wrapcheck // in-package store error surfaced verbatim } + if i < len(stores)-1 { + logging.Debug(ctx, "checkpoint: backfill target absent in store, trying next", + slog.String("checkpoint_id", checkpointID.String()), + slog.String("request_type", fmt.Sprintf("%T", req)), + slog.Int("store_index", i)) + } } return err //nolint:wrapcheck // ErrCheckpointNotFound from the final store, surfaced verbatim } @@ -209,6 +237,11 @@ func (s *kindRoutingStore) Write(ctx context.Context, req WriteRequest) error { // backfillTarget returns the checkpoint ID a backfill request updates. // ok is false for Session (a create) and unknown request types, which are not // kind-routed. +// +// WriteRequest is a closed union: any new backfill-shaped request type MUST be +// added to this switch, or it silently gets create routing — primary-only, no +// fallback — which for a pre-migration checkpoint reintroduces the discarded- +// write bug this routing exists to prevent. func backfillTarget(req WriteRequest) (id.CheckpointID, bool) { switch r := req.(type) { case SessionTranscript: @@ -248,6 +281,8 @@ func (s *kindRoutingStore) isPrimary(st PersistentStore) bool { case BackendTypeGitRefs: return st == s.refs default: + // Not a real configuration today (buildPrimary only accepts the git + // backends); backfills would bypass writer and therefore mirrors. return false } } diff --git a/cmd/entire/cli/checkpoint/routing_store_test.go b/cmd/entire/cli/checkpoint/routing_store_test.go index c9e4f29e0c..7a57a5db83 100644 --- a/cmd/entire/cli/checkpoint/routing_store_test.go +++ b/cmd/entire/cli/checkpoint/routing_store_test.go @@ -330,6 +330,90 @@ func TestKindRoutingStore_BackfillMirrorFanout(t *testing.T) { }) } +func TestKindRoutingStore_BackfillDoesNotCreateSessionsBranchOnMiss(t *testing.T) { + t.Parallel() + ctx := context.Background() + + // A refs-only repo has no v1 branch. Probing the branch store for a hex + // checkpoint that exists nowhere (e.g. a typo'd `explain --generate `) + // must report not-found WITHOUT creating the sessions branch as a side + // effect — the branch would otherwise become live (List union, pre-push). + _, repo, _ := newTestRepo(t) + branch := NewGitStore(repo, DefaultV1Refs()) + refs := newGitRefsStore(repo) + + router := newKindRoutingStore(refs, branch, refs, BackendTypeGitRefs) + + err := router.Write(ctx, SessionSummary{ + CheckpointID: id.MustCheckpointID("a1b2c3d4e5f6"), + Summary: &Summary{Intent: "orphan"}, + }) + require.ErrorIs(t, err, ErrCheckpointNotFound) + + _, refErr := repo.Reference(DefaultV1Refs().Primary, true) + require.ErrorIs(t, refErr, plumbing.ErrReferenceNotFound, + "a backfill miss must not create the sessions branch") +} + +func TestKindRoutingStore_BackfillHardErrorAbortsFallthrough(t *testing.T) { + t.Parallel() + ctx := context.Background() + + // Only ErrCheckpointNotFound may fall through — deliberately stricter than + // read routing, which falls through on any error. Redirecting a write to + // another backend after a transient primary failure could fork the data: + // the checkpoint also exists on the branch here, and the backfill must NOT + // reach it when the primary failed hard. + hexID := id.MustCheckpointID("a1b2c3d4e5f6") + _, repo, _ := newTestRepo(t) + branch := NewGitStore(repo, DefaultV1Refs()) + writeRoutingCheckpoint(t, branch, hexID, "hex-on-branch") + + failing := &fakePrimary{writeErr: errors.New("refs backend io error")} + router := newKindRoutingStore(failing, branch, failing, BackendTypeGitRefs) + + err := router.Write(ctx, SessionSummary{ + CheckpointID: hexID, + Summary: &Summary{Intent: "must not land"}, + }) + require.ErrorContains(t, err, "refs backend io error", "a hard primary error must surface verbatim") + + meta, readErr := branch.ReadSessionMetadata(ctx, hexID, 0) + require.NoError(t, readErr) + require.Nil(t, meta.Summary, "the fallback store must not receive a write after a hard primary error") +} + +func TestKindRoutingStore_BackfillPrefersRefsWhenInBothBackends(t *testing.T) { + t.Parallel() + ctx := context.Background() + + // A migrated hex checkpoint can coexist in both backends. The backfill must + // land on refs (read order under a refs primary), because refs-first reads + // would never surface a summary written to the branch copy. + hexID := id.MustCheckpointID("a1b2c3d4e5f6") + _, repo, _ := newTestRepo(t) + branch := NewGitStore(repo, DefaultV1Refs()) + refs := newGitRefsStore(repo) + writeRoutingCheckpoint(t, branch, hexID, "hex-on-branch") + writeRoutingCheckpoint(t, refs, hexID, "hex-migrated-to-refs") + + router := newKindRoutingStore(refs, branch, refs, BackendTypeGitRefs) + + require.NoError(t, router.Write(ctx, SessionSummary{ + CheckpointID: hexID, + Summary: &Summary{Intent: "lands on refs"}, + })) + + meta, err := router.ReadSessionMetadata(ctx, hexID, 0) + require.NoError(t, err) + require.NotNil(t, meta.Summary, "refs-first reads must see the backfilled summary") + assert.Equal(t, "lands on refs", meta.Summary.Intent) + + branchMeta, err := branch.ReadSessionMetadata(ctx, hexID, 0) + require.NoError(t, err) + assert.Nil(t, branchMeta.Summary, "the branch copy must not have received the backfill") +} + func TestKindRoutingStore_GetCheckpointAuthorRoutes(t *testing.T) { t.Parallel() ctx := context.Background() diff --git a/docs/architecture/ref-checkpoint-backend.md b/docs/architecture/ref-checkpoint-backend.md index 31f4988113..ef8bb5114f 100644 --- a/docs/architecture/ref-checkpoint-backend.md +++ b/docs/architecture/ref-checkpoint-backend.md @@ -120,9 +120,9 @@ A checkpoint written on another machine has no local ref. When a read misses loc `List` at the storage level is **local-refs-only** — it enumerates local refs and reads each root summary. There is no remote enumeration. -## Read routing and coexistence +## Kind routing and coexistence -`checkpoint.Open` returns a `kindRoutingStore` (`checkpoint/routing_store.go`) that resolves id-keyed reads across **both** git backends by the checkpoint's ID kind, so a repo running git-refs and git-branch side by side (or mid-migration) reads either format without reconfiguring: +`checkpoint.Open` returns a `kindRoutingStore` (`checkpoint/routing_store.go`) that resolves id-keyed reads — and backfill writes — across **both** git backends by the checkpoint's ID kind, so a repo running git-refs and git-branch side by side (or mid-migration) handles either format without reconfiguring: | ID kind | Read from | Rationale | |---------|-----------|-----------| @@ -133,7 +133,8 @@ A checkpoint written on another machine has no local ref. When a read misses loc - `List` **unions both** backends and de-dups by ID (the same checkpoint can appear in both during coexistence), keeping the most recent. - The `firstResolved` helper tries stores in priority order; a non-final store that reports absent *or* errors falls through to the next, so a transient git-refs fetch error cannot hide a checkpoint that resolves on the branch. The final store's result (hit, absent, or error) is returned verbatim. - The optional `AuthorReader` capability (`explain` relies on it) is preserved and routed by the same rules when both read stores provide it. -- **Writes are not kind-routed.** They target the configured primary (+ mirrors); the minted ID already matches the primary's format. +- **Creates (`Session`) are not kind-routed.** They target the configured primary (+ mirrors); the minted ID already matches the primary's format. +- **Backfills (`SessionSummary`, `SessionTranscript`, `CheckpointAttribution`) are kind-routed.** They update an *existing* checkpoint, which may live in either backend, so they follow the read order above, falling through to the next store only on `ErrCheckpointNotFound` (stricter than reads — a hard error aborts rather than risking a forked write). A backfill landing on the primary still fans out to mirrors; one landing on a fallback store skips mirrors (mirrors follow the primary) and logs the routing decision. All general read paths — resume, explain, attribution, blame, tokens, attach — inherit this routing for free through `checkpoint.Open`; there is no per-command config knob. @@ -202,7 +203,7 @@ When checkpoints *are* actively migrated from the branch into refs (a path that | `checkpoint/refs_naming.go` | `RefName` / `ParseRef`, `CheckpointRefPrefix` | | `checkpoint/refs_store.go` | `gitRefsStore` — per-checkpoint write/read, on-demand fetch | | `checkpoint/pushqueue.go` | Flock JSONL push-discovery queue | -| `checkpoint/routing_store.go` | `kindRoutingStore` — id-kind read routing across both backends | +| `checkpoint/routing_store.go` | `kindRoutingStore` — id-kind read + backfill-write routing across both backends | | `checkpoint/id/id.go` | `ShardFor`, `Kind`/`KindOf`, ID generation | | `checkpointpolicy/format.go` | `branch-v1` / `refs-v1` format families and read/write sets | | `settings/checkpoints.go` | `checkpoints` block parsing + env override | diff --git a/docs/architecture/sessions-and-checkpoints.md b/docs/architecture/sessions-and-checkpoints.md index b802d04735..e64064f9dd 100644 --- a/docs/architecture/sessions-and-checkpoints.md +++ b/docs/architecture/sessions-and-checkpoints.md @@ -390,8 +390,11 @@ reconfiguring: - A **hex** ID is read from the active (configured) primary first; when the primary is git-refs it also falls back to the git-branch store (a hex checkpoint may still sit on the pre-migration v1 branch, or have been migrated into refs). -- `List` unions both backends. **Writes are not kind-routed** — they go to the - configured primary (+ mirrors); the minted ID already matches the primary. +- `List` unions both backends. **Creates (`Session`) are not kind-routed** — they + go to the configured primary (+ mirrors); the minted ID already matches the + primary. **Backfills** (summary/transcript/attribution) update an existing + checkpoint and ARE kind-routed: they follow the read order, falling through to + the next store only on `ErrCheckpointNotFound`. **Generation:** - Minted by `checkpoint.GenerateCheckpointID`, which picks the format from the From d48448696be31511dec533d2e97731866fd3a34a Mon Sep 17 00:00:00 2001 From: Peyton Montei Date: Mon, 20 Jul 2026 16:15:06 -0700 Subject: [PATCH 3/3] test(explain): end-to-end regression test for the motivating --generate bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cover the full CLI path the field bug broke: auto target resolution, routed read, summary generation (stubbed provider), kind-routed backfill, and the summary persisting on the v1 branch — with a git-refs primary configured and the hex checkpoint existing only on the branch. Verified to fail on main and pass here. Closes the coverage gap the trail confidence eval flagged (no end-to-end test of the explain --generate path). Co-Authored-By: Claude Fable 5 --- cmd/entire/cli/explain_test.go | 91 ++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/cmd/entire/cli/explain_test.go b/cmd/entire/cli/explain_test.go index a17471223c..e2a27161dc 100644 --- a/cmd/entire/cli/explain_test.go +++ b/cmd/entire/cli/explain_test.go @@ -1804,6 +1804,97 @@ func TestRunExplainCheckpoint_GenerateWritesV1Store(t *testing.T) { require.Equal(t, "selected v1 intent", v1Metadata.Summary.Intent) } +// TestRunExplainAuto_GeneratePersistsHexOnBranchUnderRefsPrimary is the +// end-to-end regression test for the motivating field bug: under a git-refs +// primary, `entire checkpoint explain --generate ` for a pre-migration +// checkpoint that lives only on the v1 branch generated the AI summary and +// then discarded it (the summary backfill went refs-only), reporting +// `no checkpoint or commit found`. The whole CLI path must work: auto target +// resolution → routed read → summary generation → kind-routed backfill → +// summary persisted on the v1 branch. +func TestRunExplainAuto_GeneratePersistsHexOnBranchUnderRefsPrimary(t *testing.T) { + tmpDir := t.TempDir() + t.Chdir(tmpDir) + + testutil.InitRepo(t, tmpDir) + repo, err := git.PlainOpen(tmpDir) + require.NoError(t, err) + + wt, err := repo.Worktree() + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "test.txt"), []byte("test"), 0o644)) + _, err = wt.Add("test.txt") + require.NoError(t, err) + _, err = wt.Commit("initial commit", &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@example.com", When: time.Now()}, + }) + require.NoError(t, err) + + // git-refs is the configured primary — the field configuration in which + // the backfill was discarded. + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, ".entire"), 0o755)) + require.NoError(t, os.WriteFile( + filepath.Join(tmpDir, ".entire", "settings.json"), + []byte(`{"enabled": true, "summary_generation": {"provider": "claude-code"}, "checkpoints": {"primary": {"type": "git-refs"}}}`), + 0o644, + )) + + originalGet := getSummaryAgent + originalCLI := isSummaryCLIAvailable + originalDiscover := discoverSummaryProviders + originalGenerate := generateTranscriptSummary + t.Cleanup(func() { + getSummaryAgent = originalGet + isSummaryCLIAvailable = originalCLI + discoverSummaryProviders = originalDiscover + generateTranscriptSummary = originalGenerate + }) + + getSummaryAgent = func(name types.AgentName) (agent.Agent, error) { + return &stubTextAgent{name: name, kind: agent.AgentTypeClaudeCode}, nil + } + isSummaryCLIAvailable = func(types.AgentName) bool { return true } + discoverSummaryProviders = func(context.Context) {} + generateTranscriptSummary = func( + _ context.Context, + _ redact.RedactedBytes, + _ []string, + _ types.AgentType, + _ summarize.Generator, + ) (*checkpoint.Summary, error) { + return &checkpoint.Summary{Intent: "backfilled intent", Outcome: "backfilled outcome"}, nil + } + + // The checkpoint exists ONLY on the pre-migration v1 branch. + branchStore := checkpoint.NewGitStore(repo, checkpoint.DefaultV1Refs()) + cpID := id.MustCheckpointID("aabbccddeeff") + ctx := context.Background() + + transcript := []byte(`{"type":"user","message":{"content":[{"type":"text","text":"generate test"}]}}` + "\n" + + `{"type":"assistant","message":{"content":"done"}}` + "\n") + + require.NoError(t, branchStore.Write(ctx, checkpoint.Session{ + CheckpointID: cpID, + SessionID: "session-hex-on-branch", + Strategy: "manual-commit", + Transcript: redact.AlreadyRedacted(transcript), + AuthorName: "Test", + AuthorEmail: "test@example.com", + })) + + var buf, errBuf bytes.Buffer + err = runExplainAuto(ctx, &buf, &errBuf, cpID.String(), true, false, false, false, true, false, false, 0) + require.NoError(t, err, "generate for a hex checkpoint on the branch must succeed under a refs primary") + require.NotContains(t, errBuf.String(), "no checkpoint or commit found", + "the resolved checkpoint must not be misreported as missing") + + // The summary must be readable back from the v1 branch copy. + meta, err := branchStore.ReadSessionMetadata(ctx, cpID, 0) + require.NoError(t, err) + require.NotNil(t, meta.Summary, "the generated summary must persist on the v1 branch") + require.Equal(t, "backfilled intent", meta.Summary.Intent) +} + func TestRunExplainCheckpoint_GenerateReloadsAfterV1Write(t *testing.T) { tmpDir := t.TempDir() t.Chdir(tmpDir)