From f61602a8138ddac71459cdaa1e8b23827c00f98d Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Wed, 1 Jul 2026 16:41:05 -0400 Subject: [PATCH 01/19] fix(attribution): deterministic checkpoint metadata resolution across clones MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three clones of the same repo at the same commit could see three different 'entire why' outputs (#1551). Two residual causes after the earlier actionable-guidance work: - When the checkpoint summary resolved but the per-session records were unreadable (treeless/partial clone, gc-pruned objects), the resolver marked the metadata missing with NO reason and never attempted the remote refresh — so clones with different local blob availability silently diverged, and a multi-session checkpoint could even pick a different fallback session per clone. - Each missing checkpoint ran its own remote-fetch chain and discarded the fetch error: a file with many misses paid one network timeout per checkpoint, and the reason could not distinguish 'fetch failed' from 'the remote genuinely lacks this checkpoint'. Restructure the resolver around a memoized once-per-resolver refresh: - Any incomplete local read (summary unreadable OR session records partially/ wholly unreadable) triggers ONE durable metadata fetch; on success the store is re-pointed at a fresh repo handle and the read retried, so every clone of the same remote converges on the same answer. - The refresh outcome is memoized: many misses share one fetch (or one failure), making a single run internally consistent and bounding network attempts to one. - A retry that reads worse than the local pass (an unpushed local-only checkpoint vanishing from the remote view) keeps the local data. - MetadataMissingReason now distinguishes the three recovery situations: no fetch attempted (run this git fetch), refresh failed (the underlying error), and refresh succeeded but the remote lacks the checkpoint (ask its author to push entire/checkpoints/v1) — the last no longer suggests a pointless fetch. - The sessions-unreadable case now carries a reason on the blame (no-fetch) path too, and the resolution path emits debug logs (summary miss, sessions read x/y, refresh outcome) for field diagnosis. Closes #1551 --- cmd/entire/cli/attribution.go | 231 +++++++++++++++++---- cmd/entire/cli/attribution_refresh_test.go | 195 +++++++++++++++++ cmd/entire/cli/attribution_test.go | 10 +- 3 files changed, 390 insertions(+), 46 deletions(-) create mode 100644 cmd/entire/cli/attribution_refresh_test.go diff --git a/cmd/entire/cli/attribution.go b/cmd/entire/cli/attribution.go index 6361e675dc..c8d7c69432 100644 --- a/cmd/entire/cli/attribution.go +++ b/cmd/entire/cli/attribution.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "io" + "log/slog" "os" "os/exec" "path/filepath" @@ -19,6 +20,7 @@ import ( "github.com/entireio/cli/cmd/entire/cli/checkpoint" "github.com/entireio/cli/cmd/entire/cli/checkpoint/id" + "github.com/entireio/cli/cmd/entire/cli/logging" "github.com/entireio/cli/cmd/entire/cli/paths" "github.com/entireio/cli/cmd/entire/cli/stringutil" "github.com/entireio/cli/cmd/entire/cli/trailers" @@ -138,6 +140,17 @@ type attributionResolver struct { commitCache map[string]*object.Commit checkpointCache map[string]attributionCheckpointContext + + // Memoized remote metadata refresh. The refresh runs AT MOST ONCE per + // resolver, no matter how many checkpoints miss: getMetadataTree durably + // fetches entire/checkpoints/v1, so one refresh serves every subsequent + // read, and a failure is not retried per checkpoint (a file with many + // missing checkpoints must not pay one network timeout per checkpoint). + // This also makes a single run internally consistent: every miss in the + // run sees the same refresh outcome. + refreshAttempted bool + refreshErr error + freshRepo *git.Repository // owned by the resolver; closed in Close } func newBlameCmd() *cobra.Command { @@ -368,9 +381,15 @@ func newAttributionResolver(ctx context.Context, fetchOnMiss bool) (*attribution } func (r *attributionResolver) Close() { - if r != nil && r.repo != nil { + if r == nil { + return + } + if r.repo != nil { _ = r.repo.Close() } + if r.freshRepo != nil { + _ = r.freshRepo.Close() + } } func (r *attributionResolver) resolveLine(raw rawBlameLine, file string) attributionLine { @@ -444,34 +463,108 @@ func (r *attributionResolver) checkpointContext(cpID id.CheckpointID, file strin return ctx } +// localCheckpointRead is the outcome of one pass over the resolver's current +// store: the assembled context plus the diagnostics that decide whether a +// remote refresh could improve it and what reason to surface if not. +type localCheckpointRead struct { + ctx attributionCheckpointContext + summaryErr error + sessionsTotal int + sessionsRead int + sessionErr error // first session read error, as the representative cause +} + +// incomplete reports whether a remote refresh could plausibly improve this +// read: the checkpoint summary was unreadable, or the summary listed sessions +// whose per-session records (metadata/prompt blobs) were partially or wholly +// unreadable — the treeless-clone / gc-pruned case, where the blame tree is +// present but blobs are not. +func (l localCheckpointRead) incomplete() bool { + return l.summaryErr != nil || (l.sessionsTotal > 0 && l.sessionsRead < l.sessionsTotal) +} + +// worseThan reports whether this (post-refresh) read lost information relative +// to prev. A checkpoint that exists only locally (author hasn't pushed) can +// legitimately read worse from the refreshed store; in that case the local +// read wins. +func (l localCheckpointRead) worseThan(prev localCheckpointRead) bool { + if l.summaryErr != nil { + return prev.summaryErr == nil + } + if prev.summaryErr != nil { + return false + } + return l.sessionsRead < prev.sessionsRead +} + +// readCheckpointContext assembles the checkpoint context for a file, refreshing +// the metadata branch from the remote (once per resolver) when the local read +// is incomplete and fetchOnMiss is set. Every clone of the same remote thereby +// converges on the same answer, and when it cannot, MetadataMissingReason says +// deterministically why and how to recover. func (r *attributionResolver) readCheckpointContext(cpID id.CheckpointID, file string) attributionCheckpointContext { - ctx := attributionCheckpointContext{CheckpointID: cpID.String()} - summary, err := readAttributionCheckpointSummary(r.ctx, r.store, cpID) - if err != nil && r.fetchOnMiss { - fetched, fetchErr := r.fetchCheckpointContext(cpID, file) - if fetchErr == nil { - return fetched + logCtx := logging.WithComponent(r.ctx, "attribution") + read := r.readCheckpointContextOnce(cpID, file) + if read.incomplete() { + logging.Debug(logCtx, "checkpoint metadata incomplete locally", + slog.String("checkpoint_id", cpID.String()), + slog.Bool("summary_missing", read.summaryErr != nil), + slog.Int("sessions_total", read.sessionsTotal), + slog.Int("sessions_read", read.sessionsRead), + slog.Bool("fetch_on_miss", r.fetchOnMiss)) + if r.fetchOnMiss && r.refreshMetadataStore() == nil { + retry := r.readCheckpointContextOnce(cpID, file) + if !retry.worseThan(read) { + read = retry + } } - err = fmt.Errorf("%w (remote refresh failed: %w)", err, fetchErr) } + + if read.summaryErr != nil { + read.ctx.MetadataMissing = true + read.ctx.MetadataMissingReason = r.missReason(cpID.String(), + "checkpoint metadata was not found locally", read.summaryErr, true) + return read.ctx + } + if read.ctx.MetadataMissing { + // Sessions were listed but none could be read (per-session blobs + // absent — partial clone or pruned objects), so agent/model/prompt are + // unavailable even though the checkpoint itself resolves. + read.ctx.MetadataMissingReason = r.missReason(cpID.String(), + fmt.Sprintf("checkpoint found, but none of its %d session record(s) could be read locally", read.sessionsTotal), + read.sessionErr, false) + } + return read.ctx +} + +// readCheckpointContextOnce is a single, side-effect-free pass over the +// resolver's current store. It never fetches; refresh policy lives in +// readCheckpointContext. MetadataMissingReason is left empty — the caller +// attaches it with refresh-outcome context. +func (r *attributionResolver) readCheckpointContextOnce(cpID id.CheckpointID, file string) localCheckpointRead { + read := localCheckpointRead{ctx: attributionCheckpointContext{CheckpointID: cpID.String()}} + summary, err := readAttributionCheckpointSummary(r.ctx, r.store, cpID) if err != nil { - ctx.MetadataMissing = true - ctx.MetadataMissingReason = metadataMissingReason(r.ctx, cpID.String(), err) - return ctx + read.summaryErr = err + return read } + ctx := &read.ctx ctx.FilesTouched = normalizePathSlice(summary.FilesTouched) + read.sessionsTotal = len(summary.Sessions) selected := checkpointSessionForFile{} var fallback checkpointSessionForFile - sessionsRead := 0 matchedFile := false for i := range summary.Sessions { sessionCtx, readErr := r.readSessionForCheckpoint(cpID, i) if readErr != nil { + if read.sessionErr == nil { + read.sessionErr = readErr + } continue } - sessionsRead++ + read.sessionsRead++ if fallback.SessionID == "" { fallback = sessionCtx } @@ -501,15 +594,15 @@ func (r *attributionResolver) readCheckpointContext(cpID id.CheckpointID, file s // mean "unknown", which is not evidence of a rename, so flagging it would // print a misleading caveat (common for older metadata and attach/trail // sessions that don't populate FilesTouched). - if selected.SessionID != "" && !matchedFile && (sessionsRead > 1 || len(selected.FilesTouched) > 0) { + if selected.SessionID != "" && !matchedFile && (read.sessionsRead > 1 || len(selected.FilesTouched) > 0) { ctx.SessionFallback = true } // Sessions existed but none could be read: the per-session detail (agent, // model, prompt) is unavailable even though the checkpoint commit exists. - // Mark it missing so callers show the "trailer-level only" hint and the - // why path attempts a remote fetch. - if len(summary.Sessions) > 0 && sessionsRead == 0 { + // Mark it missing so callers show the "trailer-level only" hint; the + // caller decides whether a remote refresh should be attempted. + if read.sessionsTotal > 0 && read.sessionsRead == 0 { ctx.MetadataMissing = true } @@ -537,7 +630,7 @@ func (r *attributionResolver) readCheckpointContext(cpID id.CheckpointID, file s if len(ctx.FilesTouched) == 0 { ctx.FilesTouched = normalizePathSlice(summary.FilesTouched) } - return ctx + return read } func readAttributionCheckpointSummary(ctx context.Context, reader attributionCheckpointReader, cpID id.CheckpointID) (*checkpoint.CheckpointSummary, error) { @@ -554,40 +647,92 @@ func readAttributionCheckpointSummary(ctx context.Context, reader attributionChe return summary, nil } -func metadataMissingReason(ctx context.Context, checkpointID string, cause error) string { - reason := "checkpoint metadata was not found locally" - if cause != nil { - reason = fmt.Sprintf("%s (%v)", reason, cause) - } - if checkpointID == "" { - return fmt.Sprintf("%s. Run: %s.", reason, suggestCheckpointFetchCommand(ctx)) +// fetchAttributionMetadata refreshes the local entire/checkpoints/v1 metadata +// from the remote (checkpoint remote → treeless origin → full origin chain; see +// getMetadataTree). The fetch is durable — it updates local refs/packfiles — so +// one call serves every read that follows. Injectable for tests. +var fetchAttributionMetadata = func(ctx context.Context) error { + _, repo, err := getMetadataTree(ctx) + if repo != nil { + _ = repo.Close() } - return fmt.Sprintf("%s. Run: %s. Then re-run entire checkpoint explain %s.", reason, suggestCheckpointFetchCommand(ctx), checkpointID) + return err } -func (r *attributionResolver) fetchCheckpointContext(cpID id.CheckpointID, file string) (attributionCheckpointContext, error) { - lookup, err := newExplainCheckpointLookup(r.ctx) +// openAttributionStore opens a fresh checkpoint store over a freshly-opened +// repo. After a fetch, the resolver's original repo handle can't see the new +// packfiles (go-git's storer caches), so post-refresh reads need a new handle. +// Injectable for tests. +var openAttributionStore = func(ctx context.Context) (attributionCheckpointReader, *git.Repository, error) { + repo, err := openRepository(ctx) if err != nil { - return attributionCheckpointContext{}, err + return nil, nil, fmt.Errorf("reopen repository after metadata refresh: %w", err) } - defer lookup.Close() + stores, err := checkpoint.Open(ctx, repo, checkpoint.OpenOptions{BlobFetcher: FetchBlobsByHash}) + if err != nil { + _ = repo.Close() + return nil, nil, fmt.Errorf("reopen checkpoint store after metadata refresh: %w", err) + } + return stores.Persistent, repo, nil +} - matches, fresh := matchCheckpointPrefixWithRemoteFallback(r.ctx, io.Discard, lookup, cpID.String()) - if fresh != lookup { - defer fresh.Close() +// refreshMetadataStore fetches the metadata branch from the remote AT MOST ONCE +// per resolver and, on success, re-points the resolver's store at a fresh repo +// handle. Both the outcome and the fresh store are memoized, so a file whose +// lines miss on many checkpoints pays one refresh (or one failure), not one per +// checkpoint — and every miss in the run sees the same outcome. +func (r *attributionResolver) refreshMetadataStore() error { + if r.refreshAttempted { + return r.refreshErr } - if len(matches) != 1 { - return attributionCheckpointContext{}, checkpoint.ErrCheckpointNotFound + r.refreshAttempted = true + logCtx := logging.WithComponent(r.ctx, "attribution") + + if err := fetchAttributionMetadata(r.ctx); err != nil { + r.refreshErr = err + logging.Debug(logCtx, "checkpoint metadata remote refresh failed", + slog.String("error", err.Error())) + return r.refreshErr } + store, repo, err := openAttributionStore(r.ctx) + if err != nil { + r.refreshErr = err + logging.Debug(logCtx, "checkpoint store reopen after refresh failed", + slog.String("error", err.Error())) + return r.refreshErr + } + r.freshRepo = repo + r.store = store + logging.Debug(logCtx, "checkpoint metadata refreshed from remote") + return nil +} - oldStore := r.store - oldFetchOnMiss := r.fetchOnMiss - r.store = fresh.store - r.fetchOnMiss = false - ctx := r.readCheckpointContext(cpID, file) - r.store = oldStore - r.fetchOnMiss = oldFetchOnMiss - return ctx, nil +// missReason builds the deterministic, actionable MetadataMissingReason. The +// message distinguishes the three recovery situations the issue reporters +// couldn't tell apart: no fetch was attempted (run it yourself), the fetch +// failed (here's the error), and the fetch succeeded but the remote genuinely +// lacks the checkpoint (ask its author to push). suggestExplain is false when +// the checkpoint summary resolved (explain would not add anything for +// unreadable session records beyond what a fetch fixes). +func (r *attributionResolver) missReason(checkpointID, base string, cause error, suggestExplain bool) string { + reason := base + if cause != nil { + reason = fmt.Sprintf("%s (%v)", base, cause) + } + + switch { + case r.refreshAttempted && r.refreshErr != nil: + reason = fmt.Sprintf("%s (remote refresh failed: %v)", reason, r.refreshErr) + case r.refreshAttempted: + // Refresh succeeded and the data is still absent: fetching again won't + // help — the remote's metadata branch doesn't have it. + return reason + ". The remote's entire/checkpoints/v1 does not contain it — the checkpoint may not have been pushed yet (its author can run: git push entire/checkpoints/v1), or it predates checkpointing." + } + + if checkpointID == "" || !suggestExplain { + return fmt.Sprintf("%s. Run: %s.", reason, suggestCheckpointFetchCommand(r.ctx)) + } + return fmt.Sprintf("%s. Run: %s. Then re-run entire checkpoint explain %s.", reason, suggestCheckpointFetchCommand(r.ctx), checkpointID) } type checkpointSessionForFile struct { diff --git a/cmd/entire/cli/attribution_refresh_test.go b/cmd/entire/cli/attribution_refresh_test.go new file mode 100644 index 0000000000..67563949d0 --- /dev/null +++ b/cmd/entire/cli/attribution_refresh_test.go @@ -0,0 +1,195 @@ +package cli + +import ( + "context" + "errors" + "testing" + + "github.com/entireio/cli/cmd/entire/cli/agent" + "github.com/entireio/cli/cmd/entire/cli/checkpoint" + checkpointid "github.com/entireio/cli/cmd/entire/cli/checkpoint/id" + "github.com/go-git/go-git/v6" + "github.com/stretchr/testify/require" +) + +// Tests for issue #1551: `entire why` must resolve checkpoint metadata +// deterministically across equivalent clones — refreshing from the remote when +// the local read is incomplete, at most once per resolver, and reporting a +// precise reason (fetch failed vs not pushed vs run-the-fetch-yourself) when it +// still can't. +// +// These override the package-level fetch/reopen seams, so they must not run in +// parallel with each other. + +// overrideAttributionRefresh swaps the refresh seams for the duration of a test. +func overrideAttributionRefresh(t *testing.T, fetch func(context.Context) error, open func(context.Context) (attributionCheckpointReader, *git.Repository, error)) { + t.Helper() + oldFetch, oldOpen := fetchAttributionMetadata, openAttributionStore + fetchAttributionMetadata, openAttributionStore = fetch, open + t.Cleanup(func() { + fetchAttributionMetadata, openAttributionStore = oldFetch, oldOpen + }) +} + +func fullyReadableStub(sessionID string) *attributionCheckpointReaderStub { + return &attributionCheckpointReaderStub{ + summary: &checkpoint.CheckpointSummary{ + FilesTouched: []string{"auth.py"}, + Sessions: []checkpoint.SessionFilePaths{{Metadata: "metadata.json"}}, + }, + content: &checkpoint.SessionContent{ + Metadata: checkpoint.Metadata{ + SessionID: sessionID, + FilesTouched: []string{"auth.py"}, + Agent: agent.AgentTypeClaudeCode, + Model: "claude-test", + }, + Prompts: "Fix the authentication bug.", + }, + } +} + +// The treeless-clone / gc-pruned case: the checkpoint summary resolves but every +// per-session record is unreadable. Previously this set MetadataMissing with NO +// reason and never attempted a remote refresh — the reason must now be present +// even on the no-fetch (blame) path. +func TestReadCheckpointContextSessionsUnreadableGetsReason(t *testing.T) { + t.Parallel() + cpID := checkpointid.MustCheckpointID("aab2c3d4e5f6") + reader := &attributionCheckpointReaderStub{ + summary: &checkpoint.CheckpointSummary{ + Sessions: []checkpoint.SessionFilePaths{{Metadata: "metadata.json"}}, + }, + sessionErr: errors.New("object not found"), + } + + // fetchOnMiss=false (the blame path): no refresh, but the reason must still + // name the cause and the recovery command. + ctx := newStubAttributionResolver(reader).readCheckpointContext(cpID, "auth.py") + require.True(t, ctx.MetadataMissing) + require.Contains(t, ctx.MetadataMissingReason, "session record") + require.Contains(t, ctx.MetadataMissingReason, "object not found") + require.Contains(t, ctx.MetadataMissingReason, "git fetch ") +} + +// With fetchOnMiss, unreadable session records must trigger the remote refresh, +// and a refresh that restores the blobs must yield full metadata — the +// cross-clone convergence at the heart of #1551. +func TestReadCheckpointContextRefreshRestoresUnreadableSessions(t *testing.T) { + cpID := checkpointid.MustCheckpointID("bbb2c3d4e5f6") + broken := &attributionCheckpointReaderStub{ + summary: &checkpoint.CheckpointSummary{ + Sessions: []checkpoint.SessionFilePaths{{Metadata: "metadata.json"}}, + }, + sessionErr: errors.New("object not found"), + } + overrideAttributionRefresh(t, + func(context.Context) error { return nil }, + func(context.Context) (attributionCheckpointReader, *git.Repository, error) { + return fullyReadableStub("session-refreshed"), nil, nil + }) + + resolver := newStubAttributionResolver(broken) + resolver.fetchOnMiss = true + ctx := resolver.readCheckpointContext(cpID, "auth.py") + + require.False(t, ctx.MetadataMissing, "refresh restored the session records") + require.Empty(t, ctx.MetadataMissingReason) + require.Equal(t, "session-refreshed", ctx.SessionID) + require.Equal(t, "Claude Code", ctx.Agent) +} + +// A refresh that succeeds but still lacks the checkpoint means the remote +// genuinely doesn't have it: the reason must say "not pushed", and must NOT +// suggest a git fetch (the user just did the equivalent). +func TestReadCheckpointContextRefreshedButAbsentSaysNotPushed(t *testing.T) { + cpID := checkpointid.MustCheckpointID("ccb2c3d4e5f6") + missing := &attributionCheckpointReaderStub{readErr: errors.New("object not found")} + overrideAttributionRefresh(t, + func(context.Context) error { return nil }, + func(context.Context) (attributionCheckpointReader, *git.Repository, error) { + return missing, nil, nil + }) + + resolver := newStubAttributionResolver(missing) + resolver.fetchOnMiss = true + ctx := resolver.readCheckpointContext(cpID, "auth.py") + + require.True(t, ctx.MetadataMissing) + require.Contains(t, ctx.MetadataMissingReason, "may not have been pushed") + require.NotContains(t, ctx.MetadataMissingReason, "git fetch", "fetching again cannot help when the remote lacks the checkpoint") +} + +// The refresh must run at most once per resolver, however many checkpoints +// miss: a file whose lines reference many unfetched checkpoints must not pay +// one network attempt per checkpoint. +func TestRefreshMetadataStoreRunsAtMostOnce(t *testing.T) { + fetchCalls := 0 + overrideAttributionRefresh(t, + func(context.Context) error { fetchCalls++; return errors.New("network unreachable") }, + func(context.Context) (attributionCheckpointReader, *git.Repository, error) { + t.Fatal("openAttributionStore must not be called when the fetch fails") + return nil, nil, nil + }) + + resolver := newStubAttributionResolver(&attributionCheckpointReaderStub{readErr: errors.New("object not found")}) + resolver.fetchOnMiss = true + first := resolver.readCheckpointContext(checkpointid.MustCheckpointID("ddb2c3d4e5f6"), "auth.py") + second := resolver.readCheckpointContext(checkpointid.MustCheckpointID("eeb2c3d4e5f6"), "auth.py") + + require.Equal(t, 1, fetchCalls, "one refresh serves (or fails) the whole resolver run") + require.True(t, first.MetadataMissing) + require.True(t, second.MetadataMissing) + require.Contains(t, first.MetadataMissingReason, "remote refresh failed") + require.Contains(t, first.MetadataMissingReason, "network unreachable") + require.Contains(t, second.MetadataMissingReason, "remote refresh failed", + "the memoized refresh outcome must be reflected in every subsequent miss") +} + +// A checkpoint that reads (partially) from the LOCAL store but is absent from +// the refreshed store — the author's own unpushed checkpoint — must keep the +// local data rather than adopting the emptier remote view. +func TestReadCheckpointContextKeepsLocalWhenRefreshReadsWorse(t *testing.T) { + cpID := checkpointid.MustCheckpointID("ffb2c3d4e5f6") + // Local: summary readable, sessions unreadable (partial). Remote: nothing. + local := &attributionCheckpointReaderStub{ + summary: &checkpoint.CheckpointSummary{ + FilesTouched: []string{"auth.py"}, + Sessions: []checkpoint.SessionFilePaths{{Metadata: "metadata.json"}}, + }, + sessionErr: errors.New("object not found"), + } + overrideAttributionRefresh(t, + func(context.Context) error { return nil }, + func(context.Context) (attributionCheckpointReader, *git.Repository, error) { + return &attributionCheckpointReaderStub{readErr: errors.New("not on remote")}, nil, nil + }) + + resolver := newStubAttributionResolver(local) + resolver.fetchOnMiss = true + ctx := resolver.readCheckpointContext(cpID, "auth.py") + + require.True(t, ctx.MetadataMissing, "session records are still unreadable") + require.Equal(t, []string{"auth.py"}, ctx.FilesTouched, "local summary data must survive a worse remote read") + require.Contains(t, ctx.MetadataMissingReason, "session record") +} + +// Two resolver runs against the same stores must produce identical contexts for +// the same checkpoint — the determinism contract of #1551 at the unit level. +func TestReadCheckpointContextDeterministicAcrossResolvers(t *testing.T) { + cpID := checkpointid.MustCheckpointID("abb2c3d4e5f6") + overrideAttributionRefresh(t, + func(context.Context) error { return nil }, + func(context.Context) (attributionCheckpointReader, *git.Repository, error) { + return fullyReadableStub("session-same"), nil, nil + }) + + makeCtx := func() attributionCheckpointContext { + resolver := newStubAttributionResolver(&attributionCheckpointReaderStub{readErr: errors.New("object not found")}) + resolver.fetchOnMiss = true + return resolver.readCheckpointContext(cpID, "auth.py") + } + first := makeCtx() + second := makeCtx() + require.Equal(t, first, second) +} diff --git a/cmd/entire/cli/attribution_test.go b/cmd/entire/cli/attribution_test.go index d171dddb03..6bd263a96c 100644 --- a/cmd/entire/cli/attribution_test.go +++ b/cmd/entire/cli/attribution_test.go @@ -411,9 +411,10 @@ func TestAttributionResolverMissingMetadataIncludesReason(t *testing.T) { } type attributionCheckpointReaderStub struct { - summary *checkpoint.CheckpointSummary - content *checkpoint.SessionContent - readErr error + summary *checkpoint.CheckpointSummary + content *checkpoint.SessionContent + readErr error + sessionErr error // returned by ReadSessionMetadataAndPrompts when set } func (s *attributionCheckpointReaderStub) Read(context.Context, checkpointid.CheckpointID) (*checkpoint.CheckpointSummary, error) { @@ -424,6 +425,9 @@ func (s *attributionCheckpointReaderStub) Read(context.Context, checkpointid.Che } func (s *attributionCheckpointReaderStub) ReadSessionMetadataAndPrompts(context.Context, checkpointid.CheckpointID, int) (*checkpoint.Metadata, string, error) { + if s.sessionErr != nil { + return nil, "", s.sessionErr + } if s.content == nil { return nil, "", nil } From 2c18b9f1fed3724ae0a58ba39399402b6b826124 Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Wed, 1 Jul 2026 17:03:59 -0400 Subject: [PATCH 02/19] fix(attribution): sound refresh outcome + honest miss wording (audit round 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An adversarial audit of the determinism fix confirmed four defects; all fixed: - The refresh seam wrapped getMetadataTree, whose local-branch fallback reports success when every actual remote fetch failed (offline, no remote, auth). The resolver then confidently claimed 'the remote does not contain this checkpoint — it may not have been pushed' and suppressed the git fetch suggestion, in exactly the field conditions of #1551. The seam now calls the fetch primitives directly (checkpoint remote, then the full-content origin fetch — attribution needs session blobs, so the tree-only probe is not enough) and returns success ONLY when a remote fetch genuinely succeeded. - The sessions-unreadable-after-refresh reason reused the whole-checkpoint wording, producing the self-contradiction 'checkpoint found, but … the remote does not contain it … or it predates checkpointing'. The refreshed wording now matches the miss kind: session records still unavailable after the refresh, without claiming the just-resolved checkpoint is missing. - After one successful refresh, every later incomplete checkpoint re-read the same store twice (memoized nil triggered an identical retry). A store generation counter gates the retry on the store actually changing during the call. - Test gaps: per-index session stubbing (a multi-session checkpoint with only some records readable now provably refreshes and resolves the file-matching session); the determinism test now converges two differently-damaged clones instead of two identical ones; the success path pins one-fetch-one-reopen across misses; and a real-seam regression test (local metadata branch, no remote) fails on the pre-fix seam and passes on the fixed one. --- cmd/entire/cli/attribution.go | 59 +++++--- cmd/entire/cli/attribution_refresh_test.go | 152 +++++++++++++++++++-- cmd/entire/cli/attribution_test.go | 23 +++- 3 files changed, 207 insertions(+), 27 deletions(-) diff --git a/cmd/entire/cli/attribution.go b/cmd/entire/cli/attribution.go index c8d7c69432..b179959a96 100644 --- a/cmd/entire/cli/attribution.go +++ b/cmd/entire/cli/attribution.go @@ -151,6 +151,7 @@ type attributionResolver struct { refreshAttempted bool refreshErr error freshRepo *git.Repository // owned by the resolver; closed in Close + storeGeneration int // bumped when a refresh swaps r.store } func newBlameCmd() *cobra.Command { @@ -512,7 +513,11 @@ func (r *attributionResolver) readCheckpointContext(cpID id.CheckpointID, file s slog.Int("sessions_total", read.sessionsTotal), slog.Int("sessions_read", read.sessionsRead), slog.Bool("fetch_on_miss", r.fetchOnMiss)) - if r.fetchOnMiss && r.refreshMetadataStore() == nil { + // Retry only when the refresh actually changed the store during THIS + // call: a later checkpoint whose first pass already ran against the + // refreshed store gains nothing from an identical second pass. + genBefore := r.storeGeneration + if r.fetchOnMiss && r.refreshMetadataStore() == nil && r.storeGeneration != genBefore { retry := r.readCheckpointContextOnce(cpID, file) if !retry.worseThan(read) { read = retry @@ -648,15 +653,25 @@ func readAttributionCheckpointSummary(ctx context.Context, reader attributionChe } // fetchAttributionMetadata refreshes the local entire/checkpoints/v1 metadata -// from the remote (checkpoint remote → treeless origin → full origin chain; see -// getMetadataTree). The fetch is durable — it updates local refs/packfiles — so -// one call serves every read that follows. Injectable for tests. +// from the remote. It returns nil ONLY when a remote fetch genuinely succeeded: +// the missing-reason wording relies on this to distinguish "the remote lacks +// the checkpoint" from "we never reached the remote", so it deliberately does +// NOT use getMetadataTree, whose local-branch fallback reports success when +// every actual fetch failed (offline, no remote, auth). The fetch is durable — +// it updates local refs/packfiles — so one call serves every read that follows. +// FetchMetadataBranch is the full-content variant: attribution needs the +// per-session blobs, so the tree-only probe used elsewhere is not enough. +// Injectable for tests. var fetchAttributionMetadata = func(ctx context.Context) error { - _, repo, err := getMetadataTree(ctx) - if repo != nil { - _ = repo.Close() + crErr := FetchMetadataFromCheckpointRemote(ctx) + if crErr == nil { + return nil } - return err + originErr := FetchMetadataBranch(ctx) + if originErr == nil { + return nil + } + return fmt.Errorf("checkpoint remote: %w; origin: %w", crErr, originErr) } // openAttributionStore opens a fresh checkpoint store over a freshly-opened @@ -703,6 +718,7 @@ func (r *attributionResolver) refreshMetadataStore() error { } r.freshRepo = repo r.store = store + r.storeGeneration++ logging.Debug(logCtx, "checkpoint metadata refreshed from remote") return nil } @@ -710,11 +726,14 @@ func (r *attributionResolver) refreshMetadataStore() error { // missReason builds the deterministic, actionable MetadataMissingReason. The // message distinguishes the three recovery situations the issue reporters // couldn't tell apart: no fetch was attempted (run it yourself), the fetch -// failed (here's the error), and the fetch succeeded but the remote genuinely -// lacks the checkpoint (ask its author to push). suggestExplain is false when -// the checkpoint summary resolved (explain would not add anything for -// unreadable session records beyond what a fetch fixes). -func (r *attributionResolver) missReason(checkpointID, base string, cause error, suggestExplain bool) string { +// failed (here's the error), and the fetch succeeded but the data is still +// absent (fetching again won't help). summaryMissing selects the wording for +// that last case — a whole-checkpoint miss means the remote lacks the +// checkpoint (ask its author to push); a session-records-only miss must not +// claim that, since the summary just resolved. The explain hint is only useful +// for the whole-checkpoint miss (explain re-reads the same session records a +// fetch would restore). +func (r *attributionResolver) missReason(checkpointID, base string, cause error, summaryMissing bool) string { reason := base if cause != nil { reason = fmt.Sprintf("%s (%v)", base, cause) @@ -723,13 +742,19 @@ func (r *attributionResolver) missReason(checkpointID, base string, cause error, switch { case r.refreshAttempted && r.refreshErr != nil: reason = fmt.Sprintf("%s (remote refresh failed: %v)", reason, r.refreshErr) - case r.refreshAttempted: - // Refresh succeeded and the data is still absent: fetching again won't - // help — the remote's metadata branch doesn't have it. + case r.refreshAttempted && summaryMissing: + // Refresh succeeded and the checkpoint is still absent: fetching again + // won't help — the remote's metadata branch doesn't have it. return reason + ". The remote's entire/checkpoints/v1 does not contain it — the checkpoint may not have been pushed yet (its author can run: git push entire/checkpoints/v1), or it predates checkpointing." + case r.refreshAttempted: + // The checkpoint resolved but its session records are still unreadable + // after a successful refresh: the metadata branch was pushed without + // them or the objects are unavailable — do not claim the checkpoint + // itself is missing, and don't suggest the fetch that just ran. + return reason + ". They are still unavailable after refreshing from the remote — the metadata branch may have been pushed without them; attribution stays trailer-level." } - if checkpointID == "" || !suggestExplain { + if checkpointID == "" || !summaryMissing { return fmt.Sprintf("%s. Run: %s.", reason, suggestCheckpointFetchCommand(r.ctx)) } return fmt.Sprintf("%s. Run: %s. Then re-run entire checkpoint explain %s.", reason, suggestCheckpointFetchCommand(r.ctx), checkpointID) diff --git a/cmd/entire/cli/attribution_refresh_test.go b/cmd/entire/cli/attribution_refresh_test.go index 67563949d0..725411973f 100644 --- a/cmd/entire/cli/attribution_refresh_test.go +++ b/cmd/entire/cli/attribution_refresh_test.go @@ -174,8 +174,10 @@ func TestReadCheckpointContextKeepsLocalWhenRefreshReadsWorse(t *testing.T) { require.Contains(t, ctx.MetadataMissingReason, "session record") } -// Two resolver runs against the same stores must produce identical contexts for -// the same checkpoint — the determinism contract of #1551 at the unit level. +// The determinism contract of #1551 at the unit level: two "clones" with +// DIFFERENT local damage (one can't read the summary, the other can't read the +// session records) must converge on the identical complete context once both +// refresh from the same remote. func TestReadCheckpointContextDeterministicAcrossResolvers(t *testing.T) { cpID := checkpointid.MustCheckpointID("abb2c3d4e5f6") overrideAttributionRefresh(t, @@ -184,12 +186,144 @@ func TestReadCheckpointContextDeterministicAcrossResolvers(t *testing.T) { return fullyReadableStub("session-same"), nil, nil }) - makeCtx := func() attributionCheckpointContext { - resolver := newStubAttributionResolver(&attributionCheckpointReaderStub{readErr: errors.New("object not found")}) - resolver.fetchOnMiss = true - return resolver.readCheckpointContext(cpID, "auth.py") + cloneA := newStubAttributionResolver(&attributionCheckpointReaderStub{readErr: errors.New("object not found")}) + cloneA.fetchOnMiss = true + cloneB := newStubAttributionResolver(&attributionCheckpointReaderStub{ + summary: &checkpoint.CheckpointSummary{ + Sessions: []checkpoint.SessionFilePaths{{Metadata: "metadata.json"}}, + }, + sessionErr: errors.New("object not found"), + }) + cloneB.fetchOnMiss = true + + first := cloneA.readCheckpointContext(cpID, "auth.py") + second := cloneB.readCheckpointContext(cpID, "auth.py") + require.False(t, first.MetadataMissing) + require.Equal(t, first, second, "differently-damaged clones must converge on the remote's answer") +} + +// A multi-session checkpoint with only SOME session records readable locally is +// incomplete: the fallback session would otherwise depend on which blobs a +// clone happens to have. The refresh must run and the retry must resolve the +// file-matching session. +func TestReadCheckpointContextPartialSessionsTriggerRefresh(t *testing.T) { + cpID := checkpointid.MustCheckpointID("adb2c3d4e5f6") + twoSessions := []checkpoint.SessionFilePaths{{Metadata: "0/metadata.json"}, {Metadata: "1/metadata.json"}} + matching := &checkpoint.Metadata{ + SessionID: "session-matching", + FilesTouched: []string{"auth.py"}, + Agent: agent.AgentTypeClaudeCode, } - first := makeCtx() - second := makeCtx() - require.Equal(t, first, second) + other := &checkpoint.Metadata{ + SessionID: "session-other", + FilesTouched: []string{"unrelated.go"}, + Agent: agent.AgentTypeClaudeCode, + } + // Locally the file-matching session's record is unreadable; only the + // non-matching one reads, so a no-refresh resolver would silently pick it. + local := &attributionCheckpointReaderStub{ + summary: &checkpoint.CheckpointSummary{Sessions: twoSessions}, + sessions: []attributionSessionStub{ + {err: errors.New("object not found")}, + {meta: other, prompts: "Refactor helpers."}, + }, + } + remote := &attributionCheckpointReaderStub{ + summary: &checkpoint.CheckpointSummary{Sessions: twoSessions}, + sessions: []attributionSessionStub{ + {meta: matching, prompts: "Fix the authentication bug."}, + {meta: other, prompts: "Refactor helpers."}, + }, + } + overrideAttributionRefresh(t, + func(context.Context) error { return nil }, + func(context.Context) (attributionCheckpointReader, *git.Repository, error) { + return remote, nil, nil + }) + + resolver := newStubAttributionResolver(local) + resolver.fetchOnMiss = true + ctx := resolver.readCheckpointContext(cpID, "auth.py") + + require.False(t, ctx.MetadataMissing) + require.Equal(t, "session-matching", ctx.SessionID, "the refresh must restore the file-matching session") + require.False(t, ctx.SessionFallback, "a matched file must not be flagged as a fallback guess") +} + +// A successful refresh must also run at most once: two different missing +// checkpoints share one fetch and one store reopen. +func TestRefreshMetadataStoreSuccessRunsAtMostOnce(t *testing.T) { + fetchCalls, openCalls := 0, 0 + overrideAttributionRefresh(t, + func(context.Context) error { fetchCalls++; return nil }, + func(context.Context) (attributionCheckpointReader, *git.Repository, error) { + openCalls++ + return fullyReadableStub("session-once"), nil, nil + }) + + resolver := newStubAttributionResolver(&attributionCheckpointReaderStub{readErr: errors.New("object not found")}) + resolver.fetchOnMiss = true + first := resolver.readCheckpointContext(checkpointid.MustCheckpointID("aeb2c3d4e5f6"), "auth.py") + second := resolver.readCheckpointContext(checkpointid.MustCheckpointID("afb2c3d4e5f6"), "auth.py") + + require.Equal(t, 1, fetchCalls) + require.Equal(t, 1, openCalls) + require.False(t, first.MetadataMissing) + require.False(t, second.MetadataMissing) +} + +// Sessions unreadable even after a successful refresh: the reason must say the +// records are unavailable — not claim the checkpoint "may not have been pushed" +// or "predates checkpointing", both of which the just-read summary disproves. +func TestReadCheckpointContextSessionsUnreadableAfterRefreshWording(t *testing.T) { + cpID := checkpointid.MustCheckpointID("bcb2c3d4e5f6") + broken := &attributionCheckpointReaderStub{ + summary: &checkpoint.CheckpointSummary{ + Sessions: []checkpoint.SessionFilePaths{{Metadata: "metadata.json"}}, + }, + sessionErr: errors.New("object not found"), + } + overrideAttributionRefresh(t, + func(context.Context) error { return nil }, + func(context.Context) (attributionCheckpointReader, *git.Repository, error) { + return broken, nil, nil // remote is equally damaged + }) + + resolver := newStubAttributionResolver(broken) + resolver.fetchOnMiss = true + ctx := resolver.readCheckpointContext(cpID, "auth.py") + + require.True(t, ctx.MetadataMissing) + require.Contains(t, ctx.MetadataMissingReason, "still unavailable after refreshing") + require.NotContains(t, ctx.MetadataMissingReason, "may not have been pushed", + "the summary resolved, so the checkpoint IS on the metadata branch") + require.NotContains(t, ctx.MetadataMissingReason, "predates checkpointing") +} + +// Regression pin for the unsound-refresh bug: in a repo with a LOCAL metadata +// branch but no reachable remote, the refresh must report failure (surfacing +// the fetch error and the git fetch suggestion) — not silently fall back to the +// stale local branch and then claim the checkpoint "may not have been pushed". +// Uses the real fetchAttributionMetadata seam. +func TestRefreshWithLocalBranchButNoRemoteReportsFetchFailure(t *testing.T) { + repoRoot := newAttributionRepo(t) + // A local entire/checkpoints/v1 exists (the old getMetadataTree fallback + // would "succeed" from it); the queried checkpoint is not in it. + writeAttributionCheckpoint(t, repoRoot, "99b2c3d4e5f6", checkpoint.WriteOptions{ + SessionID: "session-local-only", + Prompts: []string{"unrelated"}, + FilesTouched: []string{"auth.py"}, + Agent: agent.AgentTypeClaudeCode, + }) + + resolver := newStubAttributionResolver(&attributionCheckpointReaderStub{readErr: errors.New("object not found")}) + resolver.fetchOnMiss = true + ctx := resolver.readCheckpointContext(checkpointid.MustCheckpointID("cdb2c3d4e5f6"), "auth.py") + + require.True(t, ctx.MetadataMissing) + require.Contains(t, ctx.MetadataMissingReason, "remote refresh failed", + "no remote is configured, so the refresh must report failure, not local-fallback success") + require.Contains(t, ctx.MetadataMissingReason, "git fetch ") + require.NotContains(t, ctx.MetadataMissingReason, "may not have been pushed", + "we never reached a remote, so nothing is known about what it contains") } diff --git a/cmd/entire/cli/attribution_test.go b/cmd/entire/cli/attribution_test.go index 6bd263a96c..120c06c4cf 100644 --- a/cmd/entire/cli/attribution_test.go +++ b/cmd/entire/cli/attribution_test.go @@ -410,11 +410,22 @@ func TestAttributionResolverMissingMetadataIncludesReason(t *testing.T) { require.Contains(t, ctx.MetadataMissingReason, "entire checkpoint explain cab2c3d4e5f6") } +// attributionSessionStub is one per-index session result for +// attributionCheckpointReaderStub.sessions. +type attributionSessionStub struct { + meta *checkpoint.Metadata + prompts string + err error +} + type attributionCheckpointReaderStub struct { summary *checkpoint.CheckpointSummary content *checkpoint.SessionContent readErr error sessionErr error // returned by ReadSessionMetadataAndPrompts when set + // sessions, when non-nil, answers ReadSessionMetadataAndPrompts by index — + // for multi-session checkpoints where only some records are readable. + sessions []attributionSessionStub } func (s *attributionCheckpointReaderStub) Read(context.Context, checkpointid.CheckpointID) (*checkpoint.CheckpointSummary, error) { @@ -424,7 +435,17 @@ func (s *attributionCheckpointReaderStub) Read(context.Context, checkpointid.Che return s.summary, nil } -func (s *attributionCheckpointReaderStub) ReadSessionMetadataAndPrompts(context.Context, checkpointid.CheckpointID, int) (*checkpoint.Metadata, string, error) { +func (s *attributionCheckpointReaderStub) ReadSessionMetadataAndPrompts(_ context.Context, _ checkpointid.CheckpointID, index int) (*checkpoint.Metadata, string, error) { + if s.sessions != nil { + if index < 0 || index >= len(s.sessions) { + return nil, "", errors.New("session index out of range") + } + entry := s.sessions[index] + if entry.err != nil { + return nil, "", entry.err + } + return entry.meta, entry.prompts, nil + } if s.sessionErr != nil { return nil, "", s.sessionErr } From 82e3c53208e08f0843c905ef23cc22be211d6659 Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Wed, 1 Jul 2026 22:41:13 -0400 Subject: [PATCH 03/19] fix(attribution): evidence-gated miss wording + authoritative-remote fetch (audit round 2) Second audit pass confirmed six diagnostic-accuracy issues; all fixed: - A configured checkpoint remote is the source of truth: its fetch failure is no longer masked by an origin fallback that may hold a stale copy (which would let a miss claim 'not pushed' about a remote we never reached). The origin leg now runs only when no checkpoint remote is configured, which also removes the 'checkpoint remote: not configured' noise from every error. - The 'not pushed' claim now requires evidence: the refreshed store must report the genuine-absence sentinel. A summary read that fails for any other reason (storage error, cancellation) after a successful refresh makes no claim about the remote. - A retry rejected as worse no longer discards what it proved: when the refreshed store reports the checkpoint absent while local data survives (an unpushed local-only checkpoint with damaged session records), the reason says the checkpoint is not on the remote instead of speculating that the metadata branch 'may have been pushed without' the session records. - Reasons are collapsed to one line (git fetch errors embed multi-line output that would break the renderer). - Stale comment still attributing the fetch to getMetadataTree reworded. Tests updated to present genuine absence via the sentinel where 'not pushed' is asserted, plus new pins: non-absence errors make no remote claim; the worse-retry reason uses the retry's proof; multi-line fetch errors collapse. --- cmd/entire/cli/attribution.go | 83 ++++++++++++++-------- cmd/entire/cli/attribution_refresh_test.go | 68 +++++++++++++++--- 2 files changed, 114 insertions(+), 37 deletions(-) diff --git a/cmd/entire/cli/attribution.go b/cmd/entire/cli/attribution.go index b179959a96..f66e69c1a3 100644 --- a/cmd/entire/cli/attribution.go +++ b/cmd/entire/cli/attribution.go @@ -20,6 +20,7 @@ import ( "github.com/entireio/cli/cmd/entire/cli/checkpoint" "github.com/entireio/cli/cmd/entire/cli/checkpoint/id" + "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/stringutil" @@ -142,10 +143,10 @@ type attributionResolver struct { checkpointCache map[string]attributionCheckpointContext // Memoized remote metadata refresh. The refresh runs AT MOST ONCE per - // resolver, no matter how many checkpoints miss: getMetadataTree durably - // fetches entire/checkpoints/v1, so one refresh serves every subsequent - // read, and a failure is not retried per checkpoint (a file with many - // missing checkpoints must not pay one network timeout per checkpoint). + // resolver, no matter how many checkpoints miss: fetchAttributionMetadata + // durably fetches entire/checkpoints/v1, so one refresh serves every + // subsequent read, and a failure is not retried per checkpoint (a file with + // many missing checkpoints must not pay one network timeout per checkpoint). // This also makes a single run internally consistent: every miss in the // run sees the same refresh outcome. refreshAttempted bool @@ -506,6 +507,7 @@ func (l localCheckpointRead) worseThan(prev localCheckpointRead) bool { func (r *attributionResolver) readCheckpointContext(cpID id.CheckpointID, file string) attributionCheckpointContext { logCtx := logging.WithComponent(r.ctx, "attribution") read := r.readCheckpointContextOnce(cpID, file) + remoteLacksCheckpoint := false if read.incomplete() { logging.Debug(logCtx, "checkpoint metadata incomplete locally", slog.String("checkpoint_id", cpID.String()), @@ -519,7 +521,14 @@ func (r *attributionResolver) readCheckpointContext(cpID id.CheckpointID, file s genBefore := r.storeGeneration if r.fetchOnMiss && r.refreshMetadataStore() == nil && r.storeGeneration != genBefore { retry := r.readCheckpointContextOnce(cpID, file) - if !retry.worseThan(read) { + if retry.worseThan(read) { + // Keep the local data, but don't discard what the retry + // proved: a refreshed store that reports not-found means the + // remote's metadata branch lacks this checkpoint entirely + // (an unpushed local-only checkpoint) — the reason must say + // that instead of speculating about missing session records. + remoteLacksCheckpoint = errors.Is(retry.summaryErr, checkpoint.ErrCheckpointNotFound) + } else { read = retry } } @@ -528,7 +537,7 @@ func (r *attributionResolver) readCheckpointContext(cpID id.CheckpointID, file s if read.summaryErr != nil { read.ctx.MetadataMissing = true read.ctx.MetadataMissingReason = r.missReason(cpID.String(), - "checkpoint metadata was not found locally", read.summaryErr, true) + "checkpoint metadata was not found locally", read.summaryErr, true, false) return read.ctx } if read.ctx.MetadataMissing { @@ -537,7 +546,7 @@ func (r *attributionResolver) readCheckpointContext(cpID id.CheckpointID, file s // unavailable even though the checkpoint itself resolves. read.ctx.MetadataMissingReason = r.missReason(cpID.String(), fmt.Sprintf("checkpoint found, but none of its %d session record(s) could be read locally", read.sessionsTotal), - read.sessionErr, false) + read.sessionErr, false, remoteLacksCheckpoint) } return read.ctx } @@ -663,15 +672,20 @@ func readAttributionCheckpointSummary(ctx context.Context, reader attributionChe // per-session blobs, so the tree-only probe used elsewhere is not enough. // Injectable for tests. var fetchAttributionMetadata = func(ctx context.Context) error { - crErr := FetchMetadataFromCheckpointRemote(ctx) - if crErr == nil { + // A configured checkpoint remote is the source of truth for checkpoint + // data: do not mask its failure with an origin fallback that may hold a + // stale copy (or no metadata branch at all) — a later miss would then + // claim "not pushed" about a remote we never reached. + if remote.Configured(ctx) { + if err := FetchMetadataFromCheckpointRemote(ctx); err != nil { + return fmt.Errorf("checkpoint remote fetch failed: %w", err) + } return nil } - originErr := FetchMetadataBranch(ctx) - if originErr == nil { - return nil + if err := FetchMetadataBranch(ctx); err != nil { + return fmt.Errorf("origin fetch failed: %w", err) } - return fmt.Errorf("checkpoint remote: %w; origin: %w", crErr, originErr) + return nil } // openAttributionStore opens a fresh checkpoint store over a freshly-opened @@ -724,16 +738,17 @@ func (r *attributionResolver) refreshMetadataStore() error { } // missReason builds the deterministic, actionable MetadataMissingReason. The -// message distinguishes the three recovery situations the issue reporters -// couldn't tell apart: no fetch was attempted (run it yourself), the fetch -// failed (here's the error), and the fetch succeeded but the data is still -// absent (fetching again won't help). summaryMissing selects the wording for -// that last case — a whole-checkpoint miss means the remote lacks the -// checkpoint (ask its author to push); a session-records-only miss must not -// claim that, since the summary just resolved. The explain hint is only useful -// for the whole-checkpoint miss (explain re-reads the same session records a -// fetch would restore). -func (r *attributionResolver) missReason(checkpointID, base string, cause error, summaryMissing bool) string { +// message distinguishes the recovery situations the issue reporters couldn't +// tell apart: no fetch was attempted (run it yourself), the fetch failed +// (here's the error), and the fetch succeeded but the data is still absent +// (fetching again won't help). For that last case the wording is only as +// strong as the evidence: "not pushed" requires the refreshed store to report +// not-found (a transient read error proves nothing about the remote), and +// remoteLacksCheckpoint carries the same proof for the session-records case +// (an unpushed local-only checkpoint). The explain hint is only useful for the +// whole-checkpoint miss; the reason is collapsed to one line because git fetch +// errors embed multi-line output. Renderers print it verbatim. +func (r *attributionResolver) missReason(checkpointID, base string, cause error, summaryMissing, remoteLacksCheckpoint bool) string { reason := base if cause != nil { reason = fmt.Sprintf("%s (%v)", base, cause) @@ -742,22 +757,32 @@ func (r *attributionResolver) missReason(checkpointID, base string, cause error, switch { case r.refreshAttempted && r.refreshErr != nil: reason = fmt.Sprintf("%s (remote refresh failed: %v)", reason, r.refreshErr) + case r.refreshAttempted && summaryMissing && errors.Is(cause, checkpoint.ErrCheckpointNotFound): + // Refresh succeeded and the refreshed store reports not-found: the + // remote's metadata branch genuinely doesn't have it. + return stringutil.CollapseWhitespace(reason + ". The remote's entire/checkpoints/v1 does not contain it — the checkpoint may not have been pushed yet (its author can run: git push entire/checkpoints/v1), or it predates checkpointing.") case r.refreshAttempted && summaryMissing: - // Refresh succeeded and the checkpoint is still absent: fetching again - // won't help — the remote's metadata branch doesn't have it. - return reason + ". The remote's entire/checkpoints/v1 does not contain it — the checkpoint may not have been pushed yet (its author can run: git push entire/checkpoints/v1), or it predates checkpointing." + // Refresh succeeded but the summary read failed for a reason other + // than absence (storage error, cancellation): make no claim about + // what the remote contains. + return stringutil.CollapseWhitespace(reason + ". The metadata branch was refreshed from the remote, but reading the checkpoint still failed — see .entire/logs for details.") + case r.refreshAttempted && remoteLacksCheckpoint: + // The summary read locally, but a successful refresh proved the + // remote lacks the checkpoint entirely: this is an unpushed + // local-only checkpoint with damaged local session records. + return stringutil.CollapseWhitespace(reason + ". This checkpoint is not on the remote's entire/checkpoints/v1 (it may not have been pushed yet), and the local session records are unreadable; attribution stays trailer-level.") case r.refreshAttempted: // The checkpoint resolved but its session records are still unreadable // after a successful refresh: the metadata branch was pushed without // them or the objects are unavailable — do not claim the checkpoint // itself is missing, and don't suggest the fetch that just ran. - return reason + ". They are still unavailable after refreshing from the remote — the metadata branch may have been pushed without them; attribution stays trailer-level." + return stringutil.CollapseWhitespace(reason + ". They are still unavailable after refreshing from the remote — the metadata branch may have been pushed without them; attribution stays trailer-level.") } if checkpointID == "" || !summaryMissing { - return fmt.Sprintf("%s. Run: %s.", reason, suggestCheckpointFetchCommand(r.ctx)) + return stringutil.CollapseWhitespace(fmt.Sprintf("%s. Run: %s.", reason, suggestCheckpointFetchCommand(r.ctx))) } - return fmt.Sprintf("%s. Run: %s. Then re-run entire checkpoint explain %s.", reason, suggestCheckpointFetchCommand(r.ctx), checkpointID) + return stringutil.CollapseWhitespace(fmt.Sprintf("%s. Run: %s. Then re-run entire checkpoint explain %s.", reason, suggestCheckpointFetchCommand(r.ctx), checkpointID)) } type checkpointSessionForFile struct { diff --git a/cmd/entire/cli/attribution_refresh_test.go b/cmd/entire/cli/attribution_refresh_test.go index 725411973f..444021f0eb 100644 --- a/cmd/entire/cli/attribution_refresh_test.go +++ b/cmd/entire/cli/attribution_refresh_test.go @@ -99,12 +99,14 @@ func TestReadCheckpointContextRefreshRestoresUnreadableSessions(t *testing.T) { require.Equal(t, "Claude Code", ctx.Agent) } -// A refresh that succeeds but still lacks the checkpoint means the remote -// genuinely doesn't have it: the reason must say "not pushed", and must NOT -// suggest a git fetch (the user just did the equivalent). +// A refresh that succeeds but still reports the checkpoint as NOT FOUND means +// the remote genuinely doesn't have it: the reason must say "not pushed", and +// must NOT suggest a git fetch (the user just did the equivalent). func TestReadCheckpointContextRefreshedButAbsentSaysNotPushed(t *testing.T) { cpID := checkpointid.MustCheckpointID("ccb2c3d4e5f6") - missing := &attributionCheckpointReaderStub{readErr: errors.New("object not found")} + // summary nil + readErr nil → Read returns (nil, nil) → the store reports + // the genuine-absence sentinel (checkpoint.ErrCheckpointNotFound). + missing := &attributionCheckpointReaderStub{} overrideAttributionRefresh(t, func(context.Context) error { return nil }, func(context.Context) (attributionCheckpointReader, *git.Repository, error) { @@ -120,6 +122,49 @@ func TestReadCheckpointContextRefreshedButAbsentSaysNotPushed(t *testing.T) { require.NotContains(t, ctx.MetadataMissingReason, "git fetch", "fetching again cannot help when the remote lacks the checkpoint") } +// A refresh that succeeds while the summary read keeps failing for a +// NON-absence reason (storage error, cancellation) proves nothing about the +// remote: the reason must not claim "not pushed". +func TestReadCheckpointContextRefreshedNonAbsenceErrorMakesNoRemoteClaim(t *testing.T) { + cpID := checkpointid.MustCheckpointID("ceb2c3d4e5f6") + broken := &attributionCheckpointReaderStub{readErr: errors.New("packfile corrupt")} + overrideAttributionRefresh(t, + func(context.Context) error { return nil }, + func(context.Context) (attributionCheckpointReader, *git.Repository, error) { + return broken, nil, nil + }) + + resolver := newStubAttributionResolver(broken) + resolver.fetchOnMiss = true + ctx := resolver.readCheckpointContext(cpID, "auth.py") + + require.True(t, ctx.MetadataMissing) + require.Contains(t, ctx.MetadataMissingReason, "packfile corrupt") + require.NotContains(t, ctx.MetadataMissingReason, "may not have been pushed", + "a read error is not evidence the remote lacks the checkpoint") +} + +// Multi-line fetch errors (git output embeds newlines) must not break the +// one-line reason rendering. +func TestMissReasonCollapsesMultilineErrors(t *testing.T) { + overrideAttributionRefresh(t, + func(context.Context) error { + return errors.New("fatal: could not read\nUsername for 'https://github.com'") + }, + func(context.Context) (attributionCheckpointReader, *git.Repository, error) { + t.Fatal("open must not run when the fetch fails") + return nil, nil, nil + }) + + resolver := newStubAttributionResolver(&attributionCheckpointReaderStub{readErr: errors.New("object not found")}) + resolver.fetchOnMiss = true + ctx := resolver.readCheckpointContext(checkpointid.MustCheckpointID("cfb2c3d4e5f6"), "auth.py") + + require.True(t, ctx.MetadataMissing) + require.Contains(t, ctx.MetadataMissingReason, "could not read") + require.NotContains(t, ctx.MetadataMissingReason, "\n", "reason must render as a single line") +} + // The refresh must run at most once per resolver, however many checkpoints // miss: a file whose lines reference many unfetched checkpoints must not pay // one network attempt per checkpoint. @@ -148,10 +193,14 @@ func TestRefreshMetadataStoreRunsAtMostOnce(t *testing.T) { // A checkpoint that reads (partially) from the LOCAL store but is absent from // the refreshed store — the author's own unpushed checkpoint — must keep the -// local data rather than adopting the emptier remote view. +// local data rather than adopting the emptier remote view, AND the reason must +// use what the retry proved: the remote lacks the checkpoint entirely, so it +// must not speculate that "the metadata branch may have been pushed without" +// the session records. func TestReadCheckpointContextKeepsLocalWhenRefreshReadsWorse(t *testing.T) { cpID := checkpointid.MustCheckpointID("ffb2c3d4e5f6") - // Local: summary readable, sessions unreadable (partial). Remote: nothing. + // Local: summary readable, sessions unreadable (partial). Remote: reports + // genuine absence (nil,nil stub → checkpoint.ErrCheckpointNotFound). local := &attributionCheckpointReaderStub{ summary: &checkpoint.CheckpointSummary{ FilesTouched: []string{"auth.py"}, @@ -162,7 +211,7 @@ func TestReadCheckpointContextKeepsLocalWhenRefreshReadsWorse(t *testing.T) { overrideAttributionRefresh(t, func(context.Context) error { return nil }, func(context.Context) (attributionCheckpointReader, *git.Repository, error) { - return &attributionCheckpointReaderStub{readErr: errors.New("not on remote")}, nil, nil + return &attributionCheckpointReaderStub{}, nil, nil }) resolver := newStubAttributionResolver(local) @@ -171,7 +220,10 @@ func TestReadCheckpointContextKeepsLocalWhenRefreshReadsWorse(t *testing.T) { require.True(t, ctx.MetadataMissing, "session records are still unreadable") require.Equal(t, []string{"auth.py"}, ctx.FilesTouched, "local summary data must survive a worse remote read") - require.Contains(t, ctx.MetadataMissingReason, "session record") + require.Contains(t, ctx.MetadataMissingReason, "not on the remote", + "the retry proved the remote lacks the checkpoint — say so") + require.NotContains(t, ctx.MetadataMissingReason, "pushed without them", + "must not speculate about remote-side truncation the retry disproved") } // The determinism contract of #1551 at the unit level: two "clones" with From 197fe44c314ad7e0da521f31c833737d36077213 Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Wed, 1 Jul 2026 23:15:54 -0400 Subject: [PATCH 04/19] fix(attribution): honest docs + no dead log pointer (audit round 3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third audit pass confirmed three residual issues; all fixed: - The refreshed-but-unreadable reason pointed users at .entire/logs, which is empty for this path at the default log level (all entries are debug-only and the post-refresh read failure was never logged at all). The pointer is dropped — the cause is already embedded in the reason — and the post-refresh outcome now gets a debug log for field diagnosis. - The resolver comments overclaimed: readCheckpointContextOnce is not 'side-effect-free / never fetches' and the memoized refresh does not bound ALL network attempts — the underlying store's on-demand BlobFetcher (pre-existing behavior shared with blame, unchanged at runtime) may still attempt per-blob fetches. Comments now state the bound covers the refresh path only, and openAttributionStore documents that keeping the fetcher is deliberate (straggler healing after a genuine refresh). - The authoritative-checkpoint-remote rule had no test on the real seam: a configured checkpoint remote whose resolution fails (no network needed) now pins that its failure surfaces as 'checkpoint remote fetch failed' with no origin fallback masking and no unfounded 'not pushed' claim. --- cmd/entire/cli/attribution.go | 36 +++++++++++++++------- cmd/entire/cli/attribution_refresh_test.go | 26 ++++++++++++++++ 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/cmd/entire/cli/attribution.go b/cmd/entire/cli/attribution.go index f66e69c1a3..4e9be01be5 100644 --- a/cmd/entire/cli/attribution.go +++ b/cmd/entire/cli/attribution.go @@ -145,10 +145,12 @@ type attributionResolver struct { // Memoized remote metadata refresh. The refresh runs AT MOST ONCE per // resolver, no matter how many checkpoints miss: fetchAttributionMetadata // durably fetches entire/checkpoints/v1, so one refresh serves every - // subsequent read, and a failure is not retried per checkpoint (a file with - // many missing checkpoints must not pay one network timeout per checkpoint). - // This also makes a single run internally consistent: every miss in the - // run sees the same refresh outcome. + // subsequent read, and a failure is not retried per checkpoint. This also + // makes a single run internally consistent: every miss in the run sees the + // same refresh outcome. Note the bound covers the REFRESH path only — the + // underlying store is opened with an on-demand BlobFetcher (pre-existing + // behavior, shared with blame), which may still attempt per-blob fetches + // for individually missing objects during reads. refreshAttempted bool refreshErr error freshRepo *git.Repository // owned by the resolver; closed in Close @@ -521,6 +523,13 @@ func (r *attributionResolver) readCheckpointContext(cpID id.CheckpointID, file s genBefore := r.storeGeneration if r.fetchOnMiss && r.refreshMetadataStore() == nil && r.storeGeneration != genBefore { retry := r.readCheckpointContextOnce(cpID, file) + if retry.incomplete() { + logging.Debug(logCtx, "checkpoint metadata still incomplete after refresh", + slog.String("checkpoint_id", cpID.String()), + slog.Bool("summary_missing", retry.summaryErr != nil), + slog.Int("sessions_read", retry.sessionsRead), + slog.Int("sessions_total", retry.sessionsTotal)) + } if retry.worseThan(read) { // Keep the local data, but don't discard what the retry // proved: a refreshed store that reports not-found means the @@ -551,10 +560,12 @@ func (r *attributionResolver) readCheckpointContext(cpID id.CheckpointID, file s return read.ctx } -// readCheckpointContextOnce is a single, side-effect-free pass over the -// resolver's current store. It never fetches; refresh policy lives in -// readCheckpointContext. MetadataMissingReason is left empty — the caller -// attaches it with refresh-outcome context. +// readCheckpointContextOnce is a single pass over the resolver's current +// store. It never triggers the metadata-branch refresh — that policy lives in +// readCheckpointContext — though the store itself may attempt on-demand +// per-blob fetches for individually missing objects (its BlobFetcher, +// pre-existing behavior shared with blame). MetadataMissingReason is left +// empty — the caller attaches it with refresh-outcome context. func (r *attributionResolver) readCheckpointContextOnce(cpID id.CheckpointID, file string) localCheckpointRead { read := localCheckpointRead{ctx: attributionCheckpointContext{CheckpointID: cpID.String()}} summary, err := readAttributionCheckpointSummary(r.ctx, r.store, cpID) @@ -691,7 +702,9 @@ var fetchAttributionMetadata = func(ctx context.Context) error { // openAttributionStore opens a fresh checkpoint store over a freshly-opened // repo. After a fetch, the resolver's original repo handle can't see the new // packfiles (go-git's storer caches), so post-refresh reads need a new handle. -// Injectable for tests. +// The BlobFetcher is deliberately kept (matching the initial store): after a +// genuine refresh it can heal individual straggler blobs the branch fetch +// missed. Injectable for tests. var openAttributionStore = func(ctx context.Context) (attributionCheckpointReader, *git.Repository, error) { repo, err := openRepository(ctx) if err != nil { @@ -764,8 +777,9 @@ func (r *attributionResolver) missReason(checkpointID, base string, cause error, case r.refreshAttempted && summaryMissing: // Refresh succeeded but the summary read failed for a reason other // than absence (storage error, cancellation): make no claim about - // what the remote contains. - return stringutil.CollapseWhitespace(reason + ". The metadata branch was refreshed from the remote, but reading the checkpoint still failed — see .entire/logs for details.") + // what the remote contains. The cause is already embedded above; no + // log pointer — this path only emits debug-level entries. + return stringutil.CollapseWhitespace(reason + ". The metadata branch was refreshed from the remote, but reading the checkpoint still failed.") case r.refreshAttempted && remoteLacksCheckpoint: // The summary read locally, but a successful refresh proved the // remote lacks the checkpoint entirely: this is an unpushed diff --git a/cmd/entire/cli/attribution_refresh_test.go b/cmd/entire/cli/attribution_refresh_test.go index 444021f0eb..9724c1722e 100644 --- a/cmd/entire/cli/attribution_refresh_test.go +++ b/cmd/entire/cli/attribution_refresh_test.go @@ -3,6 +3,8 @@ package cli import ( "context" "errors" + "os" + "path/filepath" "testing" "github.com/entireio/cli/cmd/entire/cli/agent" @@ -352,6 +354,30 @@ func TestReadCheckpointContextSessionsUnreadableAfterRefreshWording(t *testing.T require.NotContains(t, ctx.MetadataMissingReason, "predates checkpointing") } +// The authoritative-remote rule, pinned on the real seam: when a checkpoint +// remote IS configured, its fetch failure must surface as the refresh outcome — +// wrapped as a checkpoint-remote failure, with no origin fallback masking it +// and no "may not have been pushed" claim about a remote never reached. +func TestRefreshConfiguredCheckpointRemoteFailureIsAuthoritative(t *testing.T) { + repoRoot := newAttributionRepo(t) + // A configured checkpoint remote in a repo with no origin: URL resolution + // fails fast (no network), exercising the authoritative-failure path. + require.NoError(t, os.MkdirAll(filepath.Join(repoRoot, ".entire"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, ".entire", "settings.json"), + []byte(`{"enabled":true,"strategy_options":{"checkpoint_remote":{"provider":"github","repo":"org/checkpoints"}}}`), 0o644)) + + resolver := newStubAttributionResolver(&attributionCheckpointReaderStub{readErr: errors.New("object not found")}) + resolver.fetchOnMiss = true + ctx := resolver.readCheckpointContext(checkpointid.MustCheckpointID("dcb2c3d4e5f6"), "auth.py") + + require.True(t, ctx.MetadataMissing) + require.Contains(t, ctx.MetadataMissingReason, "remote refresh failed") + require.Contains(t, ctx.MetadataMissingReason, "checkpoint remote fetch failed", + "the configured checkpoint remote's failure must be the surfaced outcome, not masked by an origin fallback") + require.NotContains(t, ctx.MetadataMissingReason, "may not have been pushed", + "we never reached the checkpoint remote, so nothing is known about what it contains") +} + // Regression pin for the unsound-refresh bug: in a repo with a LOCAL metadata // branch but no reachable remote, the refresh must report failure (surfacing // the fetch error and the git fetch suggestion) — not silently fall back to the From 53bc37a88d9f51490d2109bc6a0329eb7fe53ba3 Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Wed, 1 Jul 2026 23:31:48 -0400 Subject: [PATCH 05/19] fix(attribution): close FetchURL origin-fallback loophole; hermetic remote tests (audit round 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - remote.FetchURL silently returns the ORIGIN URL when the configured checkpoint remote's URL cannot be derived (unparseable origin, unknown provider) — a 'successful' refresh could then have hit origin, faking the evidence behind the not-pushed wording. fetchAttributionMetadata now detects that resolution fallback (resolved URL == origin URL) and treats it as a refresh failure, so no claim is made about a remote never reached. - The real-seam tests cleared of environment dependence: with ENTIRE_CHECKPOINT_TOKEN set, FetchURL derives a live github.com URL with no git remote and the tests would fetch over the network with the environment's token. Both now clear the token (t.Setenv), keeping them fast and hermetic. --- cmd/entire/cli/attribution.go | 12 ++++++++++++ cmd/entire/cli/attribution_refresh_test.go | 8 ++++++++ 2 files changed, 20 insertions(+) diff --git a/cmd/entire/cli/attribution.go b/cmd/entire/cli/attribution.go index 4e9be01be5..661ad3315c 100644 --- a/cmd/entire/cli/attribution.go +++ b/cmd/entire/cli/attribution.go @@ -688,6 +688,18 @@ var fetchAttributionMetadata = func(ctx context.Context) error { // stale copy (or no metadata branch at all) — a later miss would then // claim "not pushed" about a remote we never reached. if remote.Configured(ctx) { + // remote.FetchURL itself silently falls back to the origin URL when + // the checkpoint URL cannot be derived (unparseable origin, unknown + // provider). A "successful" fetch that actually hit origin would fake + // the evidence the not-pushed wording relies on — treat that + // resolution fallback as a failure here. + url, err := remote.FetchURL(ctx) + if err != nil { + return fmt.Errorf("checkpoint remote fetch failed: %w", err) + } + if originURL, oErr := remote.GetRemoteURL(ctx, "origin"); oErr == nil && originURL == url { + return errors.New("checkpoint remote fetch failed: checkpoint_remote is configured but its URL could not be derived (resolution fell back to origin)") + } if err := FetchMetadataFromCheckpointRemote(ctx); err != nil { return fmt.Errorf("checkpoint remote fetch failed: %w", err) } diff --git a/cmd/entire/cli/attribution_refresh_test.go b/cmd/entire/cli/attribution_refresh_test.go index 9724c1722e..d8ab0a9b99 100644 --- a/cmd/entire/cli/attribution_refresh_test.go +++ b/cmd/entire/cli/attribution_refresh_test.go @@ -10,6 +10,7 @@ import ( "github.com/entireio/cli/cmd/entire/cli/agent" "github.com/entireio/cli/cmd/entire/cli/checkpoint" checkpointid "github.com/entireio/cli/cmd/entire/cli/checkpoint/id" + "github.com/entireio/cli/cmd/entire/cli/checkpoint/remote" "github.com/go-git/go-git/v6" "github.com/stretchr/testify/require" ) @@ -360,6 +361,10 @@ func TestReadCheckpointContextSessionsUnreadableAfterRefreshWording(t *testing.T // and no "may not have been pushed" claim about a remote never reached. func TestRefreshConfiguredCheckpointRemoteFailureIsAuthoritative(t *testing.T) { repoRoot := newAttributionRepo(t) + // Hermeticity: with ENTIRE_CHECKPOINT_TOKEN set, FetchURL derives a real + // github.com URL without any git remote and the test would hit the network + // with the environment's token. Clear it so resolution fails fast locally. + t.Setenv(remote.CheckpointTokenEnvVar, "") // A configured checkpoint remote in a repo with no origin: URL resolution // fails fast (no network), exercising the authoritative-failure path. require.NoError(t, os.MkdirAll(filepath.Join(repoRoot, ".entire"), 0o755)) @@ -385,6 +390,9 @@ func TestRefreshConfiguredCheckpointRemoteFailureIsAuthoritative(t *testing.T) { // Uses the real fetchAttributionMetadata seam. func TestRefreshWithLocalBranchButNoRemoteReportsFetchFailure(t *testing.T) { repoRoot := newAttributionRepo(t) + // Hermeticity: ensure a developer/CI ENTIRE_CHECKPOINT_TOKEN can't turn + // this into a network-dependent test. + t.Setenv(remote.CheckpointTokenEnvVar, "") // A local entire/checkpoints/v1 exists (the old getMetadataTree fallback // would "succeed" from it); the queried checkpoint is not in it. writeAttributionCheckpoint(t, repoRoot, "99b2c3d4e5f6", checkpoint.WriteOptions{ From 543727e1a071d19452a1b6085116f2708cbc0279 Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Wed, 1 Jul 2026 23:49:07 -0400 Subject: [PATCH 06/19] fix(attribution): explicit checkpoint-remote source signal (audit round 5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The round-4 guard inferred FetchURL's origin fallback from string equality with the origin URL — unsound in both directions: the token path rewrites the origin URL to its HTTPS form before falling back (so the fallback never string-matches and a token-based CI run could still fabricate not-pushed evidence from an origin fetch), and a checkpoint_remote legitimately naming the origin repo derives a byte-identical URL (so a genuine derivation was misreported as a fallback, permanently disabling the refresh for that configuration). Replace the heuristic with an explicit signal: FetchURLWithSource returns whether the URL was derived from the configured checkpoint_remote (mirroring PushURL's boolean); FetchURL delegates. fetchAttributionMetadata now requires usedCheckpointRemote=true before treating a configured-remote refresh as authoritative. Pins: remote-package table test for the source bool (derived, equal-URLs- legit, token-fallback, no-config, token-derived); attribution-level hermetic test that the token-path origin fallback surfaces as a refresh failure rather than fake not-pushed evidence. --- cmd/entire/cli/attribution.go | 17 ++-- cmd/entire/cli/attribution_refresh_test.go | 39 ++++++++++ cmd/entire/cli/checkpoint/remote/util.go | 36 ++++++--- cmd/entire/cli/checkpoint/remote/util_test.go | 78 +++++++++++++++++++ 4 files changed, 152 insertions(+), 18 deletions(-) diff --git a/cmd/entire/cli/attribution.go b/cmd/entire/cli/attribution.go index 661ad3315c..82b8cccdfa 100644 --- a/cmd/entire/cli/attribution.go +++ b/cmd/entire/cli/attribution.go @@ -688,16 +688,19 @@ var fetchAttributionMetadata = func(ctx context.Context) error { // stale copy (or no metadata branch at all) — a later miss would then // claim "not pushed" about a remote we never reached. if remote.Configured(ctx) { - // remote.FetchURL itself silently falls back to the origin URL when - // the checkpoint URL cannot be derived (unparseable origin, unknown - // provider). A "successful" fetch that actually hit origin would fake - // the evidence the not-pushed wording relies on — treat that - // resolution fallback as a failure here. - url, err := remote.FetchURL(ctx) + // remote.FetchURL silently falls back to the origin URL when the + // checkpoint URL cannot be derived (unparseable origin, unknown + // provider, token-path short-circuit). A "successful" fetch that + // actually hit origin would fake the evidence the not-pushed wording + // relies on — require the explicit source signal, never a URL-string + // comparison (token rewrites change origin's shape, and a + // checkpoint_remote pointing at the origin repo derives an identical + // URL legitimately). + _, usedCheckpointRemote, err := remote.FetchURLWithSource(ctx) if err != nil { return fmt.Errorf("checkpoint remote fetch failed: %w", err) } - if originURL, oErr := remote.GetRemoteURL(ctx, "origin"); oErr == nil && originURL == url { + if !usedCheckpointRemote { return errors.New("checkpoint remote fetch failed: checkpoint_remote is configured but its URL could not be derived (resolution fell back to origin)") } if err := FetchMetadataFromCheckpointRemote(ctx); err != nil { diff --git a/cmd/entire/cli/attribution_refresh_test.go b/cmd/entire/cli/attribution_refresh_test.go index d8ab0a9b99..34a3356e34 100644 --- a/cmd/entire/cli/attribution_refresh_test.go +++ b/cmd/entire/cli/attribution_refresh_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "os" + "os/exec" "path/filepath" "testing" @@ -11,6 +12,7 @@ import ( "github.com/entireio/cli/cmd/entire/cli/checkpoint" checkpointid "github.com/entireio/cli/cmd/entire/cli/checkpoint/id" "github.com/entireio/cli/cmd/entire/cli/checkpoint/remote" + "github.com/entireio/cli/cmd/entire/cli/testutil" "github.com/go-git/go-git/v6" "github.com/stretchr/testify/require" ) @@ -24,6 +26,16 @@ import ( // These override the package-level fetch/reopen seams, so they must not run in // parallel with each other. +// runAttributionGit runs a git command in dir with the isolated test env. +func runAttributionGit(t *testing.T, dir string, args ...string) { + t.Helper() + cmd := exec.CommandContext(context.Background(), "git", args...) + cmd.Dir = dir + cmd.Env = testutil.GitIsolatedEnv() + out, err := cmd.CombinedOutput() + require.NoErrorf(t, err, "git %v: %s", args, out) +} + // overrideAttributionRefresh swaps the refresh seams for the duration of a test. func overrideAttributionRefresh(t *testing.T, fetch func(context.Context) error, open func(context.Context) (attributionCheckpointReader, *git.Repository, error)) { t.Helper() @@ -383,6 +395,33 @@ func TestRefreshConfiguredCheckpointRemoteFailureIsAuthoritative(t *testing.T) { "we never reached the checkpoint remote, so nothing is known about what it contains") } +// The token-path evasion, pinned on the real seam: with ENTIRE_CHECKPOINT_TOKEN +// set and a checkpoint_remote whose provider can't be resolved, FetchURL falls +// back to a token-rewritten origin URL that no string comparison would catch. +// The explicit source signal must treat that as a refresh failure — never a +// "successful" refresh that would fabricate not-pushed evidence about a +// checkpoint remote that was never contacted. Hermetic: the guard rejects +// before any fetch runs. +func TestRefreshTokenPathOriginFallbackIsNotSuccess(t *testing.T) { + repoRoot := newAttributionRepo(t) + runAttributionGit(t, repoRoot, "remote", "add", "origin", "git@github.com:org/app.git") + require.NoError(t, os.MkdirAll(filepath.Join(repoRoot, ".entire"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, ".entire", "settings.json"), + []byte(`{"enabled":true,"strategy_options":{"checkpoint_remote":{"provider":"bitbucket","repo":"org/checkpoints"}}}`), 0o644)) + t.Setenv(remote.CheckpointTokenEnvVar, "test-token") + + resolver := newStubAttributionResolver(&attributionCheckpointReaderStub{readErr: errors.New("object not found")}) + resolver.fetchOnMiss = true + ctx := resolver.readCheckpointContext(checkpointid.MustCheckpointID("ecb2c3d4e5f6"), "auth.py") + + require.True(t, ctx.MetadataMissing) + require.Contains(t, ctx.MetadataMissingReason, "remote refresh failed") + require.Contains(t, ctx.MetadataMissingReason, "fell back to origin", + "the token-path origin fallback must surface as a failure, not fake a checkpoint-remote fetch") + require.NotContains(t, ctx.MetadataMissingReason, "may not have been pushed", + "we never reached the checkpoint remote, so nothing is known about what it contains") +} + // Regression pin for the unsound-refresh bug: in a repo with a LOCAL metadata // branch but no reachable remote, the refresh must report failure (surfacing // the fetch error and the git fetch suggestion) — not silently fall back to the diff --git a/cmd/entire/cli/checkpoint/remote/util.go b/cmd/entire/cli/checkpoint/remote/util.go index 71da074c78..80278c1d2d 100644 --- a/cmd/entire/cli/checkpoint/remote/util.go +++ b/cmd/entire/cli/checkpoint/remote/util.go @@ -38,6 +38,20 @@ type FetchURLOptions struct { // If ENTIRE_CHECKPOINT_TOKEN is set and a checkpoint remote is configured, HTTPS is // forced so the token can be used even when origin is configured via SSH. func FetchURL(ctx context.Context, opts ...FetchURLOptions) (string, error) { + url, _, err := FetchURLWithSource(ctx, opts...) + return url, err +} + +// FetchURLWithSource is FetchURL plus an explicit source signal: the boolean +// reports whether the returned URL was derived from the configured +// checkpoint_remote (true) or is an origin fallback (false). Callers whose +// correctness depends on having genuinely consulted the checkpoint remote — +// e.g. attribution's "the remote doesn't contain this checkpoint" evidence — +// must use this instead of inferring the source from the URL string, which is +// unsound in both directions (token rewrites change the origin URL's shape, +// and a checkpoint_remote pointing at the origin repo derives an identical +// URL legitimately). Mirrors PushURL's boolean. +func FetchURLWithSource(ctx context.Context, opts ...FetchURLOptions) (string, bool, error) { var opt FetchURLOptions if len(opts) > 0 { opt = opts[0] @@ -68,17 +82,17 @@ func FetchURL(ctx context.Context, opts ...FetchURLOptions) (string, error) { if err != nil { if originURL != "" { logFallback(ctx, "fetch", originURL, "load settings", err) - return originURL, nil + return originURL, false, nil } - return "", fmt.Errorf("load settings: %w", err) + return "", false, fmt.Errorf("load settings: %w", err) } config := s.GetCheckpointRemote() if config == nil { if originURL == "" { - return "", fmt.Errorf("no fetch URL found: %w", originErr) + return "", false, fmt.Errorf("no fetch URL found: %w", originErr) } - return originURL, nil + return originURL, false, nil } if withToken { @@ -89,25 +103,25 @@ func FetchURL(ctx context.Context, opts ...FetchURLOptions) (string, error) { Host: host, }, config) if err == nil { - return checkpointURL, nil + return checkpointURL, true, nil } } // In token-based execution path, short-circuit to avoid additional // change in protocol. if originURL != "" { - return originURL, nil + return originURL, false, nil } } if originURL == "" { - return "", fmt.Errorf("no fetch URL found: %w", originErr) + return "", false, fmt.Errorf("no fetch URL found: %w", originErr) } info, err := ParseURL(originURL) if err != nil { logFallback(ctx, "fetch", originURL, "parse origin remote URL", err) - return originURL, nil + return originURL, false, nil } checkpointURL, err := deriveCheckpointURLFromInfo(info, config) @@ -116,13 +130,13 @@ func FetchURL(ctx context.Context, opts ...FetchURLOptions) (string, error) { // file://). Honor the configured checkpoint_remote by targeting the // provider's canonical host over HTTPS rather than falling back to origin. if providerURL, ok := resolveProviderCheckpointURL(config, originRemote, opt.WorktreeRoot); ok { - return providerURL, nil + return providerURL, true, nil } logFallback(ctx, "fetch", originURL, "derive checkpoint remote URL", err) - return originURL, nil + return originURL, false, nil } - return checkpointURL, nil + return checkpointURL, true, nil } // PushURL returns the effective checkpoint push URL for the current repository. diff --git a/cmd/entire/cli/checkpoint/remote/util_test.go b/cmd/entire/cli/checkpoint/remote/util_test.go index d81a06ada9..8f3f0199b8 100644 --- a/cmd/entire/cli/checkpoint/remote/util_test.go +++ b/cmd/entire/cli/checkpoint/remote/util_test.go @@ -102,6 +102,84 @@ func TestFetchURL(t *testing.T) { } } +// TestFetchURLWithSource pins the explicit source signal: true only when the +// URL was derived from the configured checkpoint_remote, false for every +// origin fallback. Callers that assert facts about the checkpoint remote's +// contents (attribution's "not pushed" evidence) depend on this bool being +// sound where URL-string comparison is not: a token rewrite changes origin's +// shape, and a checkpoint_remote naming the origin repo derives an identical +// URL legitimately. +func TestFetchURLWithSource(t *testing.T) { + tests := []struct { + name string + originURL string + settingsJSON string + token string + wantURL string + wantUsed bool + }{ + { + name: "derived checkpoint url reports checkpoint remote", + originURL: "git@github.com:acme/app.git", + settingsJSON: `{"enabled":true,"strategy_options":{"checkpoint_remote":{"provider":"github","repo":"acme/checkpoints"}}}`, + wantURL: "git@github.com:acme/checkpoints.git", + wantUsed: true, + }, + { + name: "checkpoint remote naming the origin repo is a genuine derivation despite equal urls", + originURL: "git@github.com:acme/app.git", + settingsJSON: `{"enabled":true,"strategy_options":{"checkpoint_remote":{"provider":"github","repo":"acme/app"}}}`, + wantURL: "git@github.com:acme/app.git", + wantUsed: true, + }, + { + name: "token path with unknown provider falls back to origin and says so", + originURL: "git@github.com:acme/app.git", + settingsJSON: `{"enabled":true,"strategy_options":{"checkpoint_remote":{"provider":"bitbucket","repo":"acme/checkpoints"}}}`, + token: "secret-token", + wantURL: "https://github.com/acme/app.git", // token-rewritten origin, NOT the checkpoint remote + wantUsed: false, + }, + { + name: "no checkpoint remote is an origin fallback", + originURL: "https://github.com/acme/app.git", + settingsJSON: `{"enabled":true}`, + wantURL: "https://github.com/acme/app.git", + wantUsed: false, + }, + { + name: "token path with known provider derives checkpoint url", + originURL: "git@github.com:acme/app.git", + settingsJSON: `{"enabled":true,"strategy_options":{"checkpoint_remote":{"provider":"github","repo":"acme/checkpoints"}}}`, + token: "secret-token", + wantURL: "https://github.com/acme/checkpoints.git", + wantUsed: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + repoDir := t.TempDir() + testutil.InitRepo(t, repoDir) + runGit(t, repoDir, "remote", "add", "origin", tt.originURL) + writeSettings(t, repoDir, tt.settingsJSON) + t.Chdir(repoDir) + t.Setenv(CheckpointTokenEnvVar, tt.token) + + got, used, err := FetchURLWithSource(context.Background()) + if err != nil { + t.Fatalf("FetchURLWithSource() error = %v", err) + } + if got != tt.wantURL { + t.Fatalf("FetchURLWithSource() url = %q, want %q", got, tt.wantURL) + } + if used != tt.wantUsed { + t.Fatalf("FetchURLWithSource() usedCheckpointRemote = %v, want %v", used, tt.wantUsed) + } + }) + } +} + func TestFetchURL_ErrorsWhenOriginMissing(t *testing.T) { repoDir := t.TempDir() testutil.InitRepo(t, repoDir) From 81b7468734bd2cc3ce3bdef13de9437d62c00ebc Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Thu, 2 Jul 2026 00:04:18 -0400 Subject: [PATCH 07/19] fix(attribution): single-resolution refresh closes the TOCTOU window (audit round 6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - fetchAttributionMetadata resolved the authority up to four times (Configured, FetchURLWithSource, then the fetch helper's own Configured + FetchURL): any divergence between reads — a transient settings failure, a concurrent settings write — could route the fetch to origin while attribution recorded an authoritative checkpoint-remote refresh. Now: load settings once, branch on GetCheckpointRemote from that load, resolve the URL once via FetchURLWithSource, and fetch THAT verified URL directly (strategy.FetchMetadataBranch), so the fetched URL is by construction the one whose source was verified. - A settings-load failure is now a refresh failure (honest 'remote refresh failed' wording) instead of being silently mapped to 'no checkpoint remote', which would have promoted origin to the evidence source. - Stale render comment ('the remote fetch has failed') updated: the refresh may also have succeeded against a remote that lacks the data. --- cmd/entire/cli/attribution.go | 37 +++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/cmd/entire/cli/attribution.go b/cmd/entire/cli/attribution.go index 82b8cccdfa..c7631e7076 100644 --- a/cmd/entire/cli/attribution.go +++ b/cmd/entire/cli/attribution.go @@ -23,6 +23,8 @@ import ( "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/settings" + "github.com/entireio/cli/cmd/entire/cli/strategy" "github.com/entireio/cli/cmd/entire/cli/stringutil" "github.com/entireio/cli/cmd/entire/cli/trailers" @@ -687,23 +689,33 @@ var fetchAttributionMetadata = func(ctx context.Context) error { // data: do not mask its failure with an origin fallback that may hold a // stale copy (or no metadata branch at all) — a later miss would then // claim "not pushed" about a remote we never reached. - if remote.Configured(ctx) { - // remote.FetchURL silently falls back to the origin URL when the - // checkpoint URL cannot be derived (unparseable origin, unknown - // provider, token-path short-circuit). A "successful" fetch that - // actually hit origin would fake the evidence the not-pushed wording - // relies on — require the explicit source signal, never a URL-string - // comparison (token rewrites change origin's shape, and a + // Resolve the authority exactly once, then fetch the URL that was + // verified. Independently re-resolving inside the fetch helper (which + // re-reads settings and git remotes) opened a TOCTOU window where a + // transient divergence could route the fetch to origin while attribution + // recorded an authoritative checkpoint-remote refresh. A settings-load + // failure is likewise a refresh failure — not "no checkpoint remote", + // which would silently promote origin to the evidence source. + s, err := settings.Load(ctx) + if err != nil { + return fmt.Errorf("checkpoint remote fetch failed: read settings: %w", err) + } + if s.GetCheckpointRemote() != nil { + // remote.FetchURLWithSource reports whether the URL was genuinely + // derived from the configured checkpoint_remote. A "successful" fetch + // that actually hit origin would fake the evidence the not-pushed + // wording relies on — require the explicit source signal, never a + // URL-string comparison (token rewrites change origin's shape, and a // checkpoint_remote pointing at the origin repo derives an identical // URL legitimately). - _, usedCheckpointRemote, err := remote.FetchURLWithSource(ctx) + url, usedCheckpointRemote, err := remote.FetchURLWithSource(ctx) if err != nil { return fmt.Errorf("checkpoint remote fetch failed: %w", err) } if !usedCheckpointRemote { return errors.New("checkpoint remote fetch failed: checkpoint_remote is configured but its URL could not be derived (resolution fell back to origin)") } - if err := FetchMetadataFromCheckpointRemote(ctx); err != nil { + if err := strategy.FetchMetadataBranch(ctx, url); err != nil { return fmt.Errorf("checkpoint remote fetch failed: %w", err) } return nil @@ -1377,10 +1389,11 @@ func renderAttributionLineWhy(w io.Writer, file string, line attributionLine) { } // The "Full context" hint suggests `entire checkpoint explain `. Only // show it when the metadata is actually present: when it is missing, the - // remote fetch `why` already attempted has failed, so explain would fail - // the same way (the reported bug — a hint that resolves to a command that + // remote refresh `why` already attempted did not produce it (it failed, + // or the remote genuinely lacks the data), so explain would fail the + // same way (the reported bug — a hint that resolves to a command that // immediately errors). In that case the MetadataMissingReason printed - // above already gives the actionable fetch-then-explain sequence. + // above already carries the appropriate recovery guidance. if line.CheckpointID != "" && !line.MetadataMissing { fmt.Fprintf(w, "\n %s %s\n\n", sty.render(sty.dim, "Full context:"), sty.render(sty.cyan, "entire checkpoint explain "+line.CheckpointID)) } else { From b3f2532f274efbe99066f4891775c6bc17ebc774 Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Thu, 2 Jul 2026 00:22:27 -0400 Subject: [PATCH 08/19] fix(attribution): neutral wording for failed re-reads; consistent fetch suggestion (audit round 7) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A retry rejected as worse whose failure is NOT genuine absence (corrupt fetched pack, cancellation) proves nothing about the remote — the reason no longer speculates that the metadata branch 'may have been pushed without' the session records, and the retry's error is surfaced instead of dropped (mirroring the evidence-gating the whole-checkpoint miss already had). - suggestCheckpointFetchCommand no longer tells the user to fetch from origin when a configured checkpoint remote's URL could not be derived — the exact contradiction of the authoritative-remote policy the refresh just refused. It uses the explicit source signal and points at the checkpoint_remote configuration instead. --- cmd/entire/cli/attach.go | 10 ++++-- cmd/entire/cli/attribution.go | 18 ++++++++-- cmd/entire/cli/attribution_refresh_test.go | 38 ++++++++++++++++++++++ 3 files changed, 61 insertions(+), 5 deletions(-) diff --git a/cmd/entire/cli/attach.go b/cmd/entire/cli/attach.go index c1a5de2688..6e103bc00a 100644 --- a/cmd/entire/cli/attach.go +++ b/cmd/entire/cli/attach.go @@ -576,13 +576,19 @@ func checkpointPresentLocally(ctx context.Context, repo *git.Repository, refs cp } // suggestCheckpointFetchCommand returns a git fetch command the user can -// paste to pull the missing v1 metadata branch. +// paste to pull the missing v1 metadata branch. When a checkpoint remote is +// configured but its URL cannot be derived (FetchURL would silently fall back +// to origin), the suggestion points at the configuration instead: telling the +// user to fetch from origin would contradict the authoritative-remote policy +// and fetch from a remote that may not carry the metadata branch at all. func suggestCheckpointFetchCommand(ctx context.Context) string { ref := "entire/checkpoints/v1:entire/checkpoints/v1" if remote.Configured(ctx) { - if url, err := remote.FetchURL(ctx); err == nil && url != "" { + url, usedCheckpointRemote, err := remote.FetchURLWithSource(ctx) + if err == nil && usedCheckpointRemote && url != "" { return fmt.Sprintf("git fetch %s %s", url, ref) } + return "review the checkpoint_remote setting in .entire/settings.json (its URL could not be derived), then: git fetch " + ref } return "git fetch origin " + ref } diff --git a/cmd/entire/cli/attribution.go b/cmd/entire/cli/attribution.go index c7631e7076..72eadc7bd7 100644 --- a/cmd/entire/cli/attribution.go +++ b/cmd/entire/cli/attribution.go @@ -512,6 +512,7 @@ func (r *attributionResolver) readCheckpointContext(cpID id.CheckpointID, file s logCtx := logging.WithComponent(r.ctx, "attribution") read := r.readCheckpointContextOnce(cpID, file) remoteLacksCheckpoint := false + var retryReadErr error if read.incomplete() { logging.Debug(logCtx, "checkpoint metadata incomplete locally", slog.String("checkpoint_id", cpID.String()), @@ -538,7 +539,12 @@ func (r *attributionResolver) readCheckpointContext(cpID id.CheckpointID, file s // remote's metadata branch lacks this checkpoint entirely // (an unpushed local-only checkpoint) — the reason must say // that instead of speculating about missing session records. + // A NON-absence retry failure proves nothing about the + // remote; carry its error so the reason stays neutral. remoteLacksCheckpoint = errors.Is(retry.summaryErr, checkpoint.ErrCheckpointNotFound) + if !remoteLacksCheckpoint { + retryReadErr = retry.summaryErr + } } else { read = retry } @@ -548,7 +554,7 @@ func (r *attributionResolver) readCheckpointContext(cpID id.CheckpointID, file s if read.summaryErr != nil { read.ctx.MetadataMissing = true read.ctx.MetadataMissingReason = r.missReason(cpID.String(), - "checkpoint metadata was not found locally", read.summaryErr, true, false) + "checkpoint metadata was not found locally", read.summaryErr, true, false, nil) return read.ctx } if read.ctx.MetadataMissing { @@ -557,7 +563,7 @@ func (r *attributionResolver) readCheckpointContext(cpID id.CheckpointID, file s // unavailable even though the checkpoint itself resolves. read.ctx.MetadataMissingReason = r.missReason(cpID.String(), fmt.Sprintf("checkpoint found, but none of its %d session record(s) could be read locally", read.sessionsTotal), - read.sessionErr, false, remoteLacksCheckpoint) + read.sessionErr, false, remoteLacksCheckpoint, retryReadErr) } return read.ctx } @@ -788,7 +794,7 @@ func (r *attributionResolver) refreshMetadataStore() error { // (an unpushed local-only checkpoint). The explain hint is only useful for the // whole-checkpoint miss; the reason is collapsed to one line because git fetch // errors embed multi-line output. Renderers print it verbatim. -func (r *attributionResolver) missReason(checkpointID, base string, cause error, summaryMissing, remoteLacksCheckpoint bool) string { +func (r *attributionResolver) missReason(checkpointID, base string, cause error, summaryMissing, remoteLacksCheckpoint bool, retryReadErr error) string { reason := base if cause != nil { reason = fmt.Sprintf("%s (%v)", base, cause) @@ -807,6 +813,12 @@ func (r *attributionResolver) missReason(checkpointID, base string, cause error, // what the remote contains. The cause is already embedded above; no // log pointer — this path only emits debug-level entries. return stringutil.CollapseWhitespace(reason + ". The metadata branch was refreshed from the remote, but reading the checkpoint still failed.") + case r.refreshAttempted && retryReadErr != nil: + // Refresh succeeded but RE-reading this checkpoint from the refreshed + // store failed for a non-absence reason: the refreshed store was never + // successfully consulted, so make no remote claim — surface the retry + // error instead. + return stringutil.CollapseWhitespace(fmt.Sprintf("%s. The metadata branch was refreshed from the remote, but re-reading the checkpoint from the refreshed store failed (%v); attribution stays trailer-level.", reason, retryReadErr)) case r.refreshAttempted && remoteLacksCheckpoint: // The summary read locally, but a successful refresh proved the // remote lacks the checkpoint entirely: this is an unpushed diff --git a/cmd/entire/cli/attribution_refresh_test.go b/cmd/entire/cli/attribution_refresh_test.go index 34a3356e34..bdd2b325e7 100644 --- a/cmd/entire/cli/attribution_refresh_test.go +++ b/cmd/entire/cli/attribution_refresh_test.go @@ -241,6 +241,38 @@ func TestReadCheckpointContextKeepsLocalWhenRefreshReadsWorse(t *testing.T) { "must not speculate about remote-side truncation the retry disproved") } +// A rejected-worse retry whose failure is NOT genuine absence (corrupt fetched +// pack, cancellation) proves nothing about the remote: the reason must stay +// neutral and surface the retry's error rather than speculating the metadata +// branch "may have been pushed without" the session records. +func TestReadCheckpointContextRejectedRetryNonAbsenceStaysNeutral(t *testing.T) { + cpID := checkpointid.MustCheckpointID("fdb2c3d4e5f6") + local := &attributionCheckpointReaderStub{ + summary: &checkpoint.CheckpointSummary{ + FilesTouched: []string{"auth.py"}, + Sessions: []checkpoint.SessionFilePaths{{Metadata: "metadata.json"}}, + }, + sessionErr: errors.New("object not found"), + } + overrideAttributionRefresh(t, + func(context.Context) error { return nil }, + func(context.Context) (attributionCheckpointReader, *git.Repository, error) { + return &attributionCheckpointReaderStub{readErr: errors.New("packfile corrupt")}, nil, nil + }) + + resolver := newStubAttributionResolver(local) + resolver.fetchOnMiss = true + ctx := resolver.readCheckpointContext(cpID, "auth.py") + + require.True(t, ctx.MetadataMissing) + require.Contains(t, ctx.MetadataMissingReason, "packfile corrupt", + "the rejected retry's error must be surfaced, not discarded") + require.NotContains(t, ctx.MetadataMissingReason, "pushed without them", + "a failed re-read is not evidence about the remote's contents") + require.NotContains(t, ctx.MetadataMissingReason, "not on the remote", + "a non-absence failure is not proof the remote lacks the checkpoint") +} + // The determinism contract of #1551 at the unit level: two "clones" with // DIFFERENT local damage (one can't read the summary, the other can't read the // session records) must converge on the identical complete context once both @@ -420,6 +452,12 @@ func TestRefreshTokenPathOriginFallbackIsNotSuccess(t *testing.T) { "the token-path origin fallback must surface as a failure, not fake a checkpoint-remote fetch") require.NotContains(t, ctx.MetadataMissingReason, "may not have been pushed", "we never reached the checkpoint remote, so nothing is known about what it contains") + // The appended recovery suggestion must not contradict the failure by + // telling the user to fetch from the very origin URL the refresh refused. + require.NotContains(t, ctx.MetadataMissingReason, "git fetch https://github.com/org/app.git", + "suggesting a fetch from origin would contradict the authoritative-remote policy") + require.Contains(t, ctx.MetadataMissingReason, "checkpoint_remote", + "the recovery guidance should point at the configuration that failed to resolve") } // Regression pin for the unsound-refresh bug: in a repo with a LOCAL metadata From e3e391e2dc440e60bda874773c742e52b6f489bc Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Thu, 2 Jul 2026 11:55:32 -0400 Subject: [PATCH 09/19] fix(attribution): fetch suggestion honest under unreadable settings (audit round 8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit suggestCheckpointFetchCommand gated on remote.Configured, which collapses a settings-load failure to 'not configured' — so a corrupt .entire/settings.json produced a reason that simultaneously said the refresh refused origin (the authoritative remote is unknowable) and told the user to 'git fetch origin'. Load settings directly: on failure, point at the unreadable settings file (whose checkpoint_remote determines the correct remote) instead of recommending origin. --- cmd/entire/cli/attach.go | 12 +++++++++-- cmd/entire/cli/attribution_refresh_test.go | 24 ++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/cmd/entire/cli/attach.go b/cmd/entire/cli/attach.go index 6e103bc00a..104c7feac4 100644 --- a/cmd/entire/cli/attach.go +++ b/cmd/entire/cli/attach.go @@ -22,6 +22,7 @@ import ( "github.com/entireio/cli/cmd/entire/cli/logging" cliReview "github.com/entireio/cli/cmd/entire/cli/review" "github.com/entireio/cli/cmd/entire/cli/session" + "github.com/entireio/cli/cmd/entire/cli/settings" "github.com/entireio/cli/cmd/entire/cli/strategy" "github.com/entireio/cli/cmd/entire/cli/trailers" "github.com/entireio/cli/cmd/entire/cli/validation" @@ -580,10 +581,17 @@ func checkpointPresentLocally(ctx context.Context, repo *git.Repository, refs cp // configured but its URL cannot be derived (FetchURL would silently fall back // to origin), the suggestion points at the configuration instead: telling the // user to fetch from origin would contradict the authoritative-remote policy -// and fetch from a remote that may not carry the metadata branch at all. +// and fetch from a remote that may not carry the metadata branch at all. The +// same applies when settings can't be read — a checkpoint remote may be +// configured in the unreadable file, so "fetch origin" is not safe guidance +// (remote.Configured would collapse that case to "not configured"). func suggestCheckpointFetchCommand(ctx context.Context) string { ref := "entire/checkpoints/v1:entire/checkpoints/v1" - if remote.Configured(ctx) { + s, err := settings.Load(ctx) + if err != nil { + return "fix .entire/settings.json (it could not be read), then re-run; the correct fetch remote depends on its checkpoint_remote setting" + } + if s.GetCheckpointRemote() != nil { url, usedCheckpointRemote, err := remote.FetchURLWithSource(ctx) if err == nil && usedCheckpointRemote && url != "" { return fmt.Sprintf("git fetch %s %s", url, ref) diff --git a/cmd/entire/cli/attribution_refresh_test.go b/cmd/entire/cli/attribution_refresh_test.go index bdd2b325e7..98b636d9c9 100644 --- a/cmd/entire/cli/attribution_refresh_test.go +++ b/cmd/entire/cli/attribution_refresh_test.go @@ -460,6 +460,30 @@ func TestRefreshTokenPathOriginFallbackIsNotSuccess(t *testing.T) { "the recovery guidance should point at the configuration that failed to resolve") } +// Corrupt settings: the refresh refuses to promote origin to the evidence +// source, so the appended recovery suggestion must not contradict that by +// telling the user to `git fetch origin` — a checkpoint remote may well be +// configured in the unreadable file. Real seams throughout. +func TestRefreshCorruptSettingsSuggestionDoesNotSayFetchOrigin(t *testing.T) { + repoRoot := newAttributionRepo(t) + t.Setenv(remote.CheckpointTokenEnvVar, "") + require.NoError(t, os.MkdirAll(filepath.Join(repoRoot, ".entire"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, ".entire", "settings.json"), + []byte(`{not valid json`), 0o644)) + + resolver := newStubAttributionResolver(&attributionCheckpointReaderStub{readErr: errors.New("object not found")}) + resolver.fetchOnMiss = true + ctx := resolver.readCheckpointContext(checkpointid.MustCheckpointID("edb2c3d4e5f6"), "auth.py") + + require.True(t, ctx.MetadataMissing) + require.Contains(t, ctx.MetadataMissingReason, "remote refresh failed") + require.Contains(t, ctx.MetadataMissingReason, "read settings") + require.NotContains(t, ctx.MetadataMissingReason, "git fetch origin", + "cannot recommend origin when the settings that name the authoritative remote are unreadable") + require.Contains(t, ctx.MetadataMissingReason, "settings.json", + "guidance should point at the unreadable settings file") +} + // Regression pin for the unsound-refresh bug: in a repo with a LOCAL metadata // branch but no reachable remote, the refresh must report failure (surfacing // the fetch error and the git fetch suggestion) — not silently fall back to the From 5e86d17d9615126e2106dbd49b9a5bfe5e41eab3 Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Thu, 2 Jul 2026 12:25:12 -0400 Subject: [PATCH 10/19] fix(attribution): hermetic reason tests; prose suggestions render as prose (audit round 9) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Two reason tests resolved the fetch suggestion against the developer's real CWD (settings.json + git remotes) — a corrupt local settings.local.json made one fail spuriously. Both now run in a hermetic temp repo, matching the file's own conventions. - suggestCheckpointFetchCommand now returns (suggestion, isCommand): the prose fallbacks (unreadable settings, underivable checkpoint-remote URL) are no longer rendered behind a 'Run:' prefix or inside a pasteable command block, which produced 'Run: fix .entire/settings.json …, then re-run; … Then re-run …' style output. Prose composes as prose; commands keep the Run:/command-block rendering. --- cmd/entire/cli/attach.go | 45 ++++++++++++++-------- cmd/entire/cli/attribution.go | 8 +++- cmd/entire/cli/attribution_refresh_test.go | 5 ++- 3 files changed, 39 insertions(+), 19 deletions(-) diff --git a/cmd/entire/cli/attach.go b/cmd/entire/cli/attach.go index 104c7feac4..f33b79b8c5 100644 --- a/cmd/entire/cli/attach.go +++ b/cmd/entire/cli/attach.go @@ -543,9 +543,18 @@ func ensureCheckpointAvailable(ctx, logCtx context.Context, repo *git.Repository } branchDescription := "entire/checkpoints/v1 branch" + suggestion, isCommand := suggestCheckpointFetchCommand(logCtx) + if isCommand { + return repo, fmt.Errorf( + "checkpoint %s referenced by HEAD is missing from the local %s after a refresh attempt. Creating a fresh checkpoint here would overwrite the original session data on push. Run:\n\n %s\n\nthen re-run attach. If the colleague who made this commit hasn't pushed their checkpoint metadata yet, ask them to do so first", + checkpointID.String(), branchDescription, suggestion, + ) + } + // Prose guidance (unreadable settings / underivable checkpoint remote URL): + // no command block — rendering prose as a pasteable command would mislead. return repo, fmt.Errorf( - "checkpoint %s referenced by HEAD is missing from the local %s after a refresh attempt. Creating a fresh checkpoint here would overwrite the original session data on push. Run:\n\n %s\n\nthen re-run attach. If the colleague who made this commit hasn't pushed their checkpoint metadata yet, ask them to do so first", - checkpointID.String(), branchDescription, suggestCheckpointFetchCommand(logCtx), + "checkpoint %s referenced by HEAD is missing from the local %s after a refresh attempt. Creating a fresh checkpoint here would overwrite the original session data on push. %s — then re-run attach. If the colleague who made this commit hasn't pushed their checkpoint metadata yet, ask them to do so first", + checkpointID.String(), branchDescription, suggestion, ) } @@ -576,29 +585,33 @@ func checkpointPresentLocally(ctx context.Context, repo *git.Repository, refs cp return summary != nil, nil } -// suggestCheckpointFetchCommand returns a git fetch command the user can -// paste to pull the missing v1 metadata branch. When a checkpoint remote is -// configured but its URL cannot be derived (FetchURL would silently fall back -// to origin), the suggestion points at the configuration instead: telling the -// user to fetch from origin would contradict the authoritative-remote policy -// and fetch from a remote that may not carry the metadata branch at all. The -// same applies when settings can't be read — a checkpoint remote may be -// configured in the unreadable file, so "fetch origin" is not safe guidance -// (remote.Configured would collapse that case to "not configured"). -func suggestCheckpointFetchCommand(ctx context.Context) string { +// suggestCheckpointFetchCommand returns recovery guidance for a missing v1 +// metadata branch. isCommand reports whether the suggestion is a pasteable git +// command; when false it is prose and callers must not render it with a +// "Run:" prefix or inside a command block. +// +// When a checkpoint remote is configured but its URL cannot be derived +// (FetchURL would silently fall back to origin), the suggestion points at the +// configuration instead: telling the user to fetch from origin would +// contradict the authoritative-remote policy and fetch from a remote that may +// not carry the metadata branch at all. The same applies when settings can't +// be read — a checkpoint remote may be configured in the unreadable file, so +// "fetch origin" is not safe guidance (remote.Configured would collapse that +// case to "not configured"). +func suggestCheckpointFetchCommand(ctx context.Context) (suggestion string, isCommand bool) { ref := "entire/checkpoints/v1:entire/checkpoints/v1" s, err := settings.Load(ctx) if err != nil { - return "fix .entire/settings.json (it could not be read), then re-run; the correct fetch remote depends on its checkpoint_remote setting" + return ".entire/settings.json could not be read — fix it first; the correct fetch remote depends on its checkpoint_remote setting", false } if s.GetCheckpointRemote() != nil { url, usedCheckpointRemote, err := remote.FetchURLWithSource(ctx) if err == nil && usedCheckpointRemote && url != "" { - return fmt.Sprintf("git fetch %s %s", url, ref) + return fmt.Sprintf("git fetch %s %s", url, ref), true } - return "review the checkpoint_remote setting in .entire/settings.json (its URL could not be derived), then: git fetch " + ref + return "review the checkpoint_remote setting in .entire/settings.json (its URL could not be derived), then run: git fetch " + ref, false } - return "git fetch origin " + ref + return "git fetch origin " + ref, true } func resolveCheckpointID(headCommit *object.Commit) (id.CheckpointID, bool) { diff --git a/cmd/entire/cli/attribution.go b/cmd/entire/cli/attribution.go index 72eadc7bd7..665118c77f 100644 --- a/cmd/entire/cli/attribution.go +++ b/cmd/entire/cli/attribution.go @@ -832,10 +832,14 @@ func (r *attributionResolver) missReason(checkpointID, base string, cause error, return stringutil.CollapseWhitespace(reason + ". They are still unavailable after refreshing from the remote — the metadata branch may have been pushed without them; attribution stays trailer-level.") } + suggestion, isCommand := suggestCheckpointFetchCommand(r.ctx) + if isCommand { + suggestion = "Run: " + suggestion + } if checkpointID == "" || !summaryMissing { - return stringutil.CollapseWhitespace(fmt.Sprintf("%s. Run: %s.", reason, suggestCheckpointFetchCommand(r.ctx))) + return stringutil.CollapseWhitespace(fmt.Sprintf("%s. %s.", reason, suggestion)) } - return stringutil.CollapseWhitespace(fmt.Sprintf("%s. Run: %s. Then re-run entire checkpoint explain %s.", reason, suggestCheckpointFetchCommand(r.ctx), checkpointID)) + return stringutil.CollapseWhitespace(fmt.Sprintf("%s. %s. Then re-run entire checkpoint explain %s.", reason, suggestion, checkpointID)) } type checkpointSessionForFile struct { diff --git a/cmd/entire/cli/attribution_refresh_test.go b/cmd/entire/cli/attribution_refresh_test.go index 98b636d9c9..34ea24f7cb 100644 --- a/cmd/entire/cli/attribution_refresh_test.go +++ b/cmd/entire/cli/attribution_refresh_test.go @@ -69,7 +69,9 @@ func fullyReadableStub(sessionID string) *attributionCheckpointReaderStub { // reason and never attempted a remote refresh — the reason must now be present // even on the no-fetch (blame) path. func TestReadCheckpointContextSessionsUnreadableGetsReason(t *testing.T) { - t.Parallel() + // Hermetic repo: the appended fetch suggestion resolves settings + git + // remotes from CWD, which must not be the developer's real checkout. + newAttributionRepo(t) cpID := checkpointid.MustCheckpointID("aab2c3d4e5f6") reader := &attributionCheckpointReaderStub{ summary: &checkpoint.CheckpointSummary{ @@ -162,6 +164,7 @@ func TestReadCheckpointContextRefreshedNonAbsenceErrorMakesNoRemoteClaim(t *test // Multi-line fetch errors (git output embeds newlines) must not break the // one-line reason rendering. func TestMissReasonCollapsesMultilineErrors(t *testing.T) { + newAttributionRepo(t) // hermetic CWD for the appended fetch suggestion overrideAttributionRefresh(t, func(context.Context) error { return errors.New("fatal: could not read\nUsername for 'https://github.com'") From 47760d709edaca6e6b995c7174319e5e9e0ba084 Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Thu, 2 Jul 2026 13:18:17 -0400 Subject: [PATCH 11/19] fix(attribution): refs-backend-aware evidence + refreshed-store capability parity (audit round 10) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebased onto main, which added the git-refs per-checkpoint backend. Two integration hazards fixed: - openAttributionStore now opens the refreshed store with the same RefFetcher the initial store has (plus the BlobFetcher) — without it, a post-refresh retry on a git-refs-backend repo silently lost on-demand per-checkpoint ref fetching and could read strictly worse than the first pass. - The strong miss evidence ('the remote's entire/checkpoints/v1 does not contain it' / 'not on the remote') is now gated on the primary backend being the git-branch backend: the v1-branch refresh proves nothing about per-checkpoint refs, so refs-backend misses get neutral wording that says why the evidence is weaker. Backend topology is memoized per resolver. Also: TestRefreshMetadataStoreRunsAtMostOnce gets the same hermetic-CWD treatment its sibling suggestion-path tests received in round 9. --- cmd/entire/cli/attribution.go | 42 +++++++++++++++++++--- cmd/entire/cli/attribution_refresh_test.go | 27 ++++++++++++++ 2 files changed, 64 insertions(+), 5 deletions(-) diff --git a/cmd/entire/cli/attribution.go b/cmd/entire/cli/attribution.go index 665118c77f..a748477df6 100644 --- a/cmd/entire/cli/attribution.go +++ b/cmd/entire/cli/attribution.go @@ -157,6 +157,10 @@ type attributionResolver struct { refreshErr error freshRepo *git.Repository // owned by the resolver; closed in Close storeGeneration int // bumped when a refresh swaps r.store + + // Memoized backend topology (see primaryBackendIsRefs). + refsPrimaryKnown bool + refsPrimary bool } func newBlameCmd() *cobra.Command { @@ -735,15 +739,17 @@ var fetchAttributionMetadata = func(ctx context.Context) error { // openAttributionStore opens a fresh checkpoint store over a freshly-opened // repo. After a fetch, the resolver's original repo handle can't see the new // packfiles (go-git's storer caches), so post-refresh reads need a new handle. -// The BlobFetcher is deliberately kept (matching the initial store): after a -// genuine refresh it can heal individual straggler blobs the branch fetch -// missed. Injectable for tests. +// The BlobFetcher AND RefFetcher are deliberately kept identical to the initial +// store (newAttributionResolver): after a genuine refresh they heal individual +// straggler blobs / per-checkpoint refs the branch fetch missed — dropping +// either would make the retry read strictly worse than the first pass on a +// git-refs-backend repo. Injectable for tests. var openAttributionStore = func(ctx context.Context) (attributionCheckpointReader, *git.Repository, error) { repo, err := openRepository(ctx) if err != nil { return nil, nil, fmt.Errorf("reopen repository after metadata refresh: %w", err) } - stores, err := checkpoint.Open(ctx, repo, checkpoint.OpenOptions{BlobFetcher: FetchBlobsByHash}) + stores, err := checkpoint.Open(ctx, repo, checkpoint.OpenOptions{BlobFetcher: FetchBlobsByHash, RefFetcher: FetchCheckpointRef}) if err != nil { _ = repo.Close() return nil, nil, fmt.Errorf("reopen checkpoint store after metadata refresh: %w", err) @@ -751,6 +757,25 @@ var openAttributionStore = func(ctx context.Context) (attributionCheckpointReade return stores.Persistent, repo, nil } +// primaryBackendIsRefs reports (memoized) whether the repo's primary checkpoint +// backend is the git-refs per-checkpoint store. The v1-branch refresh proves +// nothing about per-checkpoint refs, so the strong "the remote's +// entire/checkpoints/v1 does not contain it / not on the remote" evidence is +// only sound for the git-branch backend; refs-backend misses use neutral +// wording. A config load error counts as the default git-branch backend. +func (r *attributionResolver) primaryBackendIsRefs() bool { + if r.refsPrimaryKnown { + return r.refsPrimary + } + r.refsPrimaryKnown = true + cfg, err := settings.LoadCheckpointsConfig(r.ctx) + if err != nil { + return false + } + r.refsPrimary = checkpoint.PrimaryIsRefs(cfg) + return r.refsPrimary +} + // refreshMetadataStore fetches the metadata branch from the remote AT MOST ONCE // per resolver and, on success, re-points the resolver's store at a fresh repo // handle. Both the outcome and the fresh store are memoized, so a file whose @@ -803,6 +828,10 @@ func (r *attributionResolver) missReason(checkpointID, base string, cause error, switch { case r.refreshAttempted && r.refreshErr != nil: reason = fmt.Sprintf("%s (remote refresh failed: %v)", reason, r.refreshErr) + case r.refreshAttempted && summaryMissing && errors.Is(cause, checkpoint.ErrCheckpointNotFound) && r.primaryBackendIsRefs(): + // git-refs primary: the v1-branch refresh proves nothing about + // per-checkpoint refs, so make no claim about what the remote holds. + return stringutil.CollapseWhitespace(reason + ". The metadata branch was refreshed, but this repo's primary checkpoint backend stores per-checkpoint refs, which that refresh may not cover — the checkpoint may not have been pushed yet, or its ref could not be fetched; attribution stays trailer-level.") case r.refreshAttempted && summaryMissing && errors.Is(cause, checkpoint.ErrCheckpointNotFound): // Refresh succeeded and the refreshed store reports not-found: the // remote's metadata branch genuinely doesn't have it. @@ -819,11 +848,14 @@ func (r *attributionResolver) missReason(checkpointID, base string, cause error, // successfully consulted, so make no remote claim — surface the retry // error instead. return stringutil.CollapseWhitespace(fmt.Sprintf("%s. The metadata branch was refreshed from the remote, but re-reading the checkpoint from the refreshed store failed (%v); attribution stays trailer-level.", reason, retryReadErr)) - case r.refreshAttempted && remoteLacksCheckpoint: + case r.refreshAttempted && remoteLacksCheckpoint && !r.primaryBackendIsRefs(): // The summary read locally, but a successful refresh proved the // remote lacks the checkpoint entirely: this is an unpushed // local-only checkpoint with damaged local session records. return stringutil.CollapseWhitespace(reason + ". This checkpoint is not on the remote's entire/checkpoints/v1 (it may not have been pushed yet), and the local session records are unreadable; attribution stays trailer-level.") + case r.refreshAttempted && r.primaryBackendIsRefs(): + // git-refs primary: no speculation about the v1 branch's contents. + return stringutil.CollapseWhitespace(reason + ". They are still unavailable after refreshing; attribution stays trailer-level.") case r.refreshAttempted: // The checkpoint resolved but its session records are still unreadable // after a successful refresh: the metadata branch was pushed without diff --git a/cmd/entire/cli/attribution_refresh_test.go b/cmd/entire/cli/attribution_refresh_test.go index 34ea24f7cb..1dff997185 100644 --- a/cmd/entire/cli/attribution_refresh_test.go +++ b/cmd/entire/cli/attribution_refresh_test.go @@ -12,6 +12,7 @@ import ( "github.com/entireio/cli/cmd/entire/cli/checkpoint" checkpointid "github.com/entireio/cli/cmd/entire/cli/checkpoint/id" "github.com/entireio/cli/cmd/entire/cli/checkpoint/remote" + "github.com/entireio/cli/cmd/entire/cli/settings" "github.com/entireio/cli/cmd/entire/cli/testutil" "github.com/go-git/go-git/v6" "github.com/stretchr/testify/require" @@ -139,6 +140,31 @@ func TestReadCheckpointContextRefreshedButAbsentSaysNotPushed(t *testing.T) { require.NotContains(t, ctx.MetadataMissingReason, "git fetch", "fetching again cannot help when the remote lacks the checkpoint") } +// git-refs primary backend: the v1-branch refresh proves nothing about +// per-checkpoint refs, so even a refreshed-store not-found must NOT claim +// "the remote's entire/checkpoints/v1 does not contain it". +func TestReadCheckpointContextRefsBackendMakesNoBranchClaim(t *testing.T) { + newAttributionRepo(t) + t.Setenv(settings.EnvCheckpointsPrimary, "git-refs") + + missing := &attributionCheckpointReaderStub{} // sentinel not-found + overrideAttributionRefresh(t, + func(context.Context) error { return nil }, + func(context.Context) (attributionCheckpointReader, *git.Repository, error) { + return missing, nil, nil + }) + + resolver := newStubAttributionResolver(missing) + resolver.fetchOnMiss = true + ctx := resolver.readCheckpointContext(checkpointid.MustCheckpointID("fab2c3d4e5f6"), "auth.py") + + require.True(t, ctx.MetadataMissing) + require.NotContains(t, ctx.MetadataMissingReason, "does not contain it", + "the branch refresh cannot prove anything about per-checkpoint refs") + require.Contains(t, ctx.MetadataMissingReason, "per-checkpoint refs", + "the reason should say why the evidence is weaker on this backend") +} + // A refresh that succeeds while the summary read keeps failing for a // NON-absence reason (storage error, cancellation) proves nothing about the // remote: the reason must not claim "not pushed". @@ -187,6 +213,7 @@ func TestMissReasonCollapsesMultilineErrors(t *testing.T) { // miss: a file whose lines reference many unfetched checkpoints must not pay // one network attempt per checkpoint. func TestRefreshMetadataStoreRunsAtMostOnce(t *testing.T) { + newAttributionRepo(t) // hermetic CWD for the appended fetch suggestion fetchCalls := 0 overrideAttributionRefresh(t, func(context.Context) error { fetchCalls++; return errors.New("network unreachable") }, From 5e84fb0e7f2531dea2861a127ff684c0124ceed1 Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Thu, 2 Jul 2026 13:35:02 -0400 Subject: [PATCH 12/19] test(attribution): pin checkpoint backend in evidence-wording tests (audit round 11) Round 10's backend gate made missReason consult the checkpoint topology (ENTIRE_CHECKPOINTS_PRIMARY env, else CWD settings), which the evidence-wording tests inherited from the ambient environment: under ENTIRE_CHECKPOINTS_PRIMARY=git-refs, KeepsLocalWhenRefreshReadsWorse failed deterministically (neutral refs wording instead of the asserted git-branch claim) and siblings silently pinned a different branch than their names claim. Pin t.Setenv(EnvCheckpointsPrimary, "git-branch") in every test that asserts backend-gated wording, and add the refs-backend twin of the worse-retry case (neutral wording, no 'not on the remote' claim), which had no coverage. --- cmd/entire/cli/attribution_refresh_test.go | 37 ++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/cmd/entire/cli/attribution_refresh_test.go b/cmd/entire/cli/attribution_refresh_test.go index 1dff997185..e374874673 100644 --- a/cmd/entire/cli/attribution_refresh_test.go +++ b/cmd/entire/cli/attribution_refresh_test.go @@ -121,6 +121,8 @@ func TestReadCheckpointContextRefreshRestoresUnreadableSessions(t *testing.T) { // the remote genuinely doesn't have it: the reason must say "not pushed", and // must NOT suggest a git fetch (the user just did the equivalent). func TestReadCheckpointContextRefreshedButAbsentSaysNotPushed(t *testing.T) { + // Pin the backend: the asserted wording is the git-branch evidence branch. + t.Setenv(settings.EnvCheckpointsPrimary, "git-branch") cpID := checkpointid.MustCheckpointID("ccb2c3d4e5f6") // summary nil + readErr nil → Read returns (nil, nil) → the store reports // the genuine-absence sentinel (checkpoint.ErrCheckpointNotFound). @@ -165,6 +167,35 @@ func TestReadCheckpointContextRefsBackendMakesNoBranchClaim(t *testing.T) { "the reason should say why the evidence is weaker on this backend") } +// refs-backend twin of KeepsLocalWhenRefreshReadsWorse: even when the retry +// proves the refreshed store lacks the checkpoint, the git-refs backend gets +// neutral wording (the branch refresh cannot support a "not on the remote" +// claim about per-checkpoint refs). +func TestReadCheckpointContextRefsBackendWorseRetryStaysNeutral(t *testing.T) { + newAttributionRepo(t) + t.Setenv(settings.EnvCheckpointsPrimary, "git-refs") + local := &attributionCheckpointReaderStub{ + summary: &checkpoint.CheckpointSummary{ + Sessions: []checkpoint.SessionFilePaths{{Metadata: "metadata.json"}}, + }, + sessionErr: errors.New("object not found"), + } + overrideAttributionRefresh(t, + func(context.Context) error { return nil }, + func(context.Context) (attributionCheckpointReader, *git.Repository, error) { + return &attributionCheckpointReaderStub{}, nil, nil // sentinel not-found + }) + + resolver := newStubAttributionResolver(local) + resolver.fetchOnMiss = true + ctx := resolver.readCheckpointContext(checkpointid.MustCheckpointID("fbb2c3d4e5f6"), "auth.py") + + require.True(t, ctx.MetadataMissing) + require.Contains(t, ctx.MetadataMissingReason, "still unavailable after refreshing") + require.NotContains(t, ctx.MetadataMissingReason, "not on the remote", + "the branch refresh cannot prove per-checkpoint-ref absence") +} + // A refresh that succeeds while the summary read keeps failing for a // NON-absence reason (storage error, cancellation) proves nothing about the // remote: the reason must not claim "not pushed". @@ -243,6 +274,8 @@ func TestRefreshMetadataStoreRunsAtMostOnce(t *testing.T) { // must not speculate that "the metadata branch may have been pushed without" // the session records. func TestReadCheckpointContextKeepsLocalWhenRefreshReadsWorse(t *testing.T) { + // Pin the backend: the asserted wording is the git-branch evidence branch. + t.Setenv(settings.EnvCheckpointsPrimary, "git-branch") cpID := checkpointid.MustCheckpointID("ffb2c3d4e5f6") // Local: summary readable, sessions unreadable (partial). Remote: reports // genuine absence (nil,nil stub → checkpoint.ErrCheckpointNotFound). @@ -276,6 +309,8 @@ func TestReadCheckpointContextKeepsLocalWhenRefreshReadsWorse(t *testing.T) { // neutral and surface the retry's error rather than speculating the metadata // branch "may have been pushed without" the session records. func TestReadCheckpointContextRejectedRetryNonAbsenceStaysNeutral(t *testing.T) { + // Pin the backend so the retryReadErr branch (not the refs branch) fires. + t.Setenv(settings.EnvCheckpointsPrimary, "git-branch") cpID := checkpointid.MustCheckpointID("fdb2c3d4e5f6") local := &attributionCheckpointReaderStub{ summary: &checkpoint.CheckpointSummary{ @@ -405,6 +440,8 @@ func TestRefreshMetadataStoreSuccessRunsAtMostOnce(t *testing.T) { // records are unavailable — not claim the checkpoint "may not have been pushed" // or "predates checkpointing", both of which the just-read summary disproves. func TestReadCheckpointContextSessionsUnreadableAfterRefreshWording(t *testing.T) { + // Pin the backend: the asserted wording is the git-branch branch. + t.Setenv(settings.EnvCheckpointsPrimary, "git-branch") cpID := checkpointid.MustCheckpointID("bcb2c3d4e5f6") broken := &attributionCheckpointReaderStub{ summary: &checkpoint.CheckpointSummary{ From 21c804fc1c3eb774aac20b8e542d739685d90214 Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Thu, 2 Jul 2026 14:10:22 -0400 Subject: [PATCH 13/19] fix(attribution): refs-primary soft branch fetch; unknown backend fails to neutral (audit round 12) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - primaryBackendIsRefs now distinguishes 'config unreadable' from a known backend: an unreadable checkpoints config supports neither backend's claim, so such misses get generic neutral wording instead of the strongest git-branch claim (the previous failure direction overclaimed — inconsistent with the branch's own evidence discipline). The v1-speculation catch-all is likewise gated on a KNOWN git-branch backend. - On a KNOWN git-refs-primary repo, a failing v1-branch fetch is now a soft outcome: the branch is not that backend's primary mechanism (a refs-only remote may not carry it), and the reopened store's per-checkpoint RefFetcher does the real work — previously every miss in the run surfaced 'remote refresh failed: … entire/checkpoints/v1 …' plus a suggestion guaranteed to fail for the same reason. Pins: refs-primary + failing branch fetch (real fetch seam, hermetic repo with no origin) yields the refs wording with no 'remote refresh failed'; an invalid checkpoints block yields neutral wording with neither backend's claim. --- cmd/entire/cli/attribution.go | 115 ++++++++++++++++++--- cmd/entire/cli/attribution_refresh_test.go | 60 +++++++++++ 2 files changed, 160 insertions(+), 15 deletions(-) diff --git a/cmd/entire/cli/attribution.go b/cmd/entire/cli/attribution.go index a748477df6..24da834c54 100644 --- a/cmd/entire/cli/attribution.go +++ b/cmd/entire/cli/attribution.go @@ -21,6 +21,7 @@ import ( "github.com/entireio/cli/cmd/entire/cli/checkpoint" "github.com/entireio/cli/cmd/entire/cli/checkpoint/id" "github.com/entireio/cli/cmd/entire/cli/checkpoint/remote" + "github.com/entireio/cli/cmd/entire/cli/interactive" "github.com/entireio/cli/cmd/entire/cli/logging" "github.com/entireio/cli/cmd/entire/cli/paths" "github.com/entireio/cli/cmd/entire/cli/settings" @@ -159,8 +160,9 @@ type attributionResolver struct { storeGeneration int // bumped when a refresh swaps r.store // Memoized backend topology (see primaryBackendIsRefs). - refsPrimaryKnown bool - refsPrimary bool + refsPrimaryKnown bool + refsPrimary bool + refsPrimaryUnknown bool // config unreadable: no backend claim is supported } func newBlameCmd() *cobra.Command { @@ -195,6 +197,7 @@ func newBlameCmd() *cobra.Command { func newWhyCmd() *cobra.Command { var jsonFlag bool var lineFlag string + var tuiFlag bool cmd := &cobra.Command{ Use: "why [:line]", @@ -203,18 +206,20 @@ func newWhyCmd() *cobra.Command { // --help` keep working normally. Hidden: true, Short: "Show why a line exists", - Long: "Explain the commit, checkpoint, prompt, and session behind a file or line.\n\nTarget a specific line with :12 or the --line flag.", + Long: "Explain the commit, checkpoint, prompt, and session behind a file or line.\n\nTarget a specific line with :12 or the --line flag.\n\nUse --tui to browse the file interactively (TTY only; non-interactive runs fall back to the plain output).", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { return runAttributionWhy(cmd.Context(), cmd.OutOrStdout(), args[0], attributionWhyOptions{ LineFlag: lineFlag, JSON: jsonFlag, + TUI: tuiFlag, }) }, } cmd.Flags().StringVar(&lineFlag, "line", "", "Explain a specific line, for example 12 (same as :12)") cmd.Flags().BoolVar(&jsonFlag, "json", false, "Output explanation as JSON") + cmd.Flags().BoolVar(&tuiFlag, "tui", false, "Browse line attribution in an interactive viewer (TTY only)") return cmd } @@ -227,6 +232,7 @@ type attributionBlameOptions struct { type attributionWhyOptions struct { LineFlag string JSON bool + TUI bool } func runAttributionBlame(ctx context.Context, w io.Writer, file string, opts attributionBlameOptions) error { @@ -287,6 +293,18 @@ func runAttributionWhy(ctx context.Context, w io.Writer, target string, opts att return err } + // Interactive browsing is strictly opt-in AND TTY-gated (agent-safe CLI + // fallbacks): a non-interactive run with --tui falls through to the same + // deterministic plain/JSON output below, which carries the full + // information. --json wins over --tui so scripted callers never block. + if opts.TUI && !opts.JSON && interactive.IsTerminalWriter(w) && !IsAccessibleMode() { + startLine := 0 + if hasLine { + startLine = line + } + return runWhyTUI(result, attributionRepoFullName(ctx), shouldUseColor(w), startLine) + } + if !hasLine { if opts.JSON { return writeJSON(w, result) @@ -726,16 +744,40 @@ var fetchAttributionMetadata = func(ctx context.Context) error { return errors.New("checkpoint remote fetch failed: checkpoint_remote is configured but its URL could not be derived (resolution fell back to origin)") } if err := strategy.FetchMetadataBranch(ctx, url); err != nil { + if attributionRefsPrimarySoftensBranchFetch(ctx, err) { + return nil + } return fmt.Errorf("checkpoint remote fetch failed: %w", err) } return nil } if err := FetchMetadataBranch(ctx); err != nil { + if attributionRefsPrimarySoftensBranchFetch(ctx, err) { + return nil + } return fmt.Errorf("origin fetch failed: %w", err) } return nil } +// attributionRefsPrimarySoftensBranchFetch reports whether a failed v1-branch +// fetch should be treated as a soft outcome: on a git-refs-primary repo the +// branch is not the backend's primary mechanism (a refs-only remote may not +// carry it at all), and the reopened store's per-checkpoint RefFetcher does the +// real work — surfacing the branch failure would pin every miss in the run to +// "remote refresh failed: … entire/checkpoints/v1 …" plus a suggestion that +// cannot succeed. Only a KNOWN refs-primary topology softens the failure. +func attributionRefsPrimarySoftensBranchFetch(ctx context.Context, fetchErr error) bool { + cfg, cfgErr := settings.LoadCheckpointsConfig(ctx) + if cfgErr != nil || !checkpoint.PrimaryIsRefs(cfg) { + return false + } + logging.Debug(logging.WithComponent(ctx, "attribution"), + "v1 branch fetch failed on a git-refs-primary repo; treating as soft (per-checkpoint refs are fetched on demand)", + slog.String("error", fetchErr.Error())) + return true +} + // openAttributionStore opens a fresh checkpoint store over a freshly-opened // repo. After a fetch, the resolver's original repo handle can't see the new // packfiles (go-git's storer caches), so post-refresh reads need a new handle. @@ -762,18 +804,21 @@ var openAttributionStore = func(ctx context.Context) (attributionCheckpointReade // nothing about per-checkpoint refs, so the strong "the remote's // entire/checkpoints/v1 does not contain it / not on the remote" evidence is // only sound for the git-branch backend; refs-backend misses use neutral -// wording. A config load error counts as the default git-branch backend. -func (r *attributionResolver) primaryBackendIsRefs() bool { +// wording. known=false means the config could not be read: neither backend's +// claim is then supported, so callers must fall back to generic neutral +// wording — an unreadable config must never select the strongest claims. +func (r *attributionResolver) primaryBackendIsRefs() (isRefs, known bool) { if r.refsPrimaryKnown { - return r.refsPrimary + return r.refsPrimary, !r.refsPrimaryUnknown } r.refsPrimaryKnown = true cfg, err := settings.LoadCheckpointsConfig(r.ctx) if err != nil { - return false + r.refsPrimaryUnknown = true + return false, false } r.refsPrimary = checkpoint.PrimaryIsRefs(cfg) - return r.refsPrimary + return r.refsPrimary, true } // refreshMetadataStore fetches the metadata branch from the remote AT MOST ONCE @@ -828,14 +873,18 @@ func (r *attributionResolver) missReason(checkpointID, base string, cause error, switch { case r.refreshAttempted && r.refreshErr != nil: reason = fmt.Sprintf("%s (remote refresh failed: %v)", reason, r.refreshErr) - case r.refreshAttempted && summaryMissing && errors.Is(cause, checkpoint.ErrCheckpointNotFound) && r.primaryBackendIsRefs(): + case r.refreshAttempted && summaryMissing && errors.Is(cause, checkpoint.ErrCheckpointNotFound) && missBackendIsRefs(r): // git-refs primary: the v1-branch refresh proves nothing about // per-checkpoint refs, so make no claim about what the remote holds. return stringutil.CollapseWhitespace(reason + ". The metadata branch was refreshed, but this repo's primary checkpoint backend stores per-checkpoint refs, which that refresh may not cover — the checkpoint may not have been pushed yet, or its ref could not be fetched; attribution stays trailer-level.") - case r.refreshAttempted && summaryMissing && errors.Is(cause, checkpoint.ErrCheckpointNotFound): + case r.refreshAttempted && summaryMissing && errors.Is(cause, checkpoint.ErrCheckpointNotFound) && missBackendIsBranch(r): // Refresh succeeded and the refreshed store reports not-found: the // remote's metadata branch genuinely doesn't have it. return stringutil.CollapseWhitespace(reason + ". The remote's entire/checkpoints/v1 does not contain it — the checkpoint may not have been pushed yet (its author can run: git push entire/checkpoints/v1), or it predates checkpointing.") + case r.refreshAttempted && summaryMissing && errors.Is(cause, checkpoint.ErrCheckpointNotFound): + // Backend unknown (config unreadable): neither the branch claim nor + // the refs wording is supported — generic neutral text. + return stringutil.CollapseWhitespace(reason + ". The metadata branch was refreshed from the remote, but the checkpoint is still unavailable; attribution stays trailer-level.") case r.refreshAttempted && summaryMissing: // Refresh succeeded but the summary read failed for a reason other // than absence (storage error, cancellation): make no claim about @@ -848,20 +897,21 @@ func (r *attributionResolver) missReason(checkpointID, base string, cause error, // successfully consulted, so make no remote claim — surface the retry // error instead. return stringutil.CollapseWhitespace(fmt.Sprintf("%s. The metadata branch was refreshed from the remote, but re-reading the checkpoint from the refreshed store failed (%v); attribution stays trailer-level.", reason, retryReadErr)) - case r.refreshAttempted && remoteLacksCheckpoint && !r.primaryBackendIsRefs(): + case r.refreshAttempted && remoteLacksCheckpoint && missBackendIsBranch(r): // The summary read locally, but a successful refresh proved the // remote lacks the checkpoint entirely: this is an unpushed // local-only checkpoint with damaged local session records. return stringutil.CollapseWhitespace(reason + ". This checkpoint is not on the remote's entire/checkpoints/v1 (it may not have been pushed yet), and the local session records are unreadable; attribution stays trailer-level.") - case r.refreshAttempted && r.primaryBackendIsRefs(): - // git-refs primary: no speculation about the v1 branch's contents. - return stringutil.CollapseWhitespace(reason + ". They are still unavailable after refreshing; attribution stays trailer-level.") - case r.refreshAttempted: + case r.refreshAttempted && missBackendIsBranch(r): // The checkpoint resolved but its session records are still unreadable // after a successful refresh: the metadata branch was pushed without // them or the objects are unavailable — do not claim the checkpoint // itself is missing, and don't suggest the fetch that just ran. return stringutil.CollapseWhitespace(reason + ". They are still unavailable after refreshing from the remote — the metadata branch may have been pushed without them; attribution stays trailer-level.") + case r.refreshAttempted: + // git-refs primary or unknown backend: no speculation about the v1 + // branch's contents. + return stringutil.CollapseWhitespace(reason + ". They are still unavailable after refreshing; attribution stays trailer-level.") } suggestion, isCommand := suggestCheckpointFetchCommand(r.ctx) @@ -1209,6 +1259,12 @@ func attributionLineMarker(line attributionLine) string { // renderAttributionMarkerLegend prints a one-line legend explaining the blame // markers, but only for the markers actually present in the table. func renderAttributionMarkerLegend(w io.Writer, sty statusStyles, lines []attributionLine) { + // Always explain the authorship tags — insiders found [MX] and [HU] + // confusing without it. Wording is deliberate (see whyMarkerLegend): + // [AI] means fully agent-authored checkpoint work, [MX] means agent work + // with human edits mixed in at commit, [HU] means no agent checkpoint. + fmt.Fprintf(w, " %s\n", sty.render(sty.dim, whyMarkerLegend)) + approximate, ambiguous := false, false for _, line := range lines { switch attributionLineMarker(line) { @@ -1667,6 +1723,21 @@ func shortCheckpointSession(line attributionLine) string { return line.CheckpointID + "/" + shortSessionID(line.SessionID) } +// attributionRepoFullName derives "owner/repo" from the origin remote for +// entire.io hyperlinks in the why TUI. Best-effort decoration: any failure +// (no origin, unparseable URL) returns "" and links are simply omitted. +func attributionRepoFullName(ctx context.Context) string { + originURL, err := remote.GetRemoteURL(ctx, "origin") + if err != nil || originURL == "" { + return "" + } + info, err := remote.ParseURL(originURL) + if err != nil || info.Owner == "" || info.Repo == "" { + return "" + } + return info.Owner + "/" + info.Repo +} + func shortSessionID(sessionID string) string { if len(sessionID) <= 8 { return sessionID @@ -1720,3 +1791,17 @@ func writeJSON(w io.Writer, value any) error { } return nil } + +// missBackendIsBranch / missBackendIsRefs are the evidence gates for +// missReason: strong git-branch claims require a KNOWN git-branch backend, and +// refs-specific wording requires a KNOWN git-refs backend. An unreadable +// config supports neither, routing the miss to generic neutral wording. +func missBackendIsBranch(r *attributionResolver) bool { + isRefs, known := r.primaryBackendIsRefs() + return known && !isRefs +} + +func missBackendIsRefs(r *attributionResolver) bool { + isRefs, known := r.primaryBackendIsRefs() + return known && isRefs +} diff --git a/cmd/entire/cli/attribution_refresh_test.go b/cmd/entire/cli/attribution_refresh_test.go index e374874673..5954bf3aef 100644 --- a/cmd/entire/cli/attribution_refresh_test.go +++ b/cmd/entire/cli/attribution_refresh_test.go @@ -196,6 +196,66 @@ func TestReadCheckpointContextRefsBackendWorseRetryStaysNeutral(t *testing.T) { "the branch refresh cannot prove per-checkpoint-ref absence") } +// refs-primary + failing v1-branch fetch: the branch is not that backend's +// primary mechanism, so the failure is soft — the refresh proceeds (reopened +// store's RefFetcher does the real work) and misses get the refs wording, not +// a sticky "remote refresh failed: … entire/checkpoints/v1 …" plus a +// suggestion that cannot succeed. Uses the real fetch seam (hermetic repo has +// no origin, so the v1 fetch genuinely fails). +func TestRefsPrimarySoftensFailingBranchFetch(t *testing.T) { + newAttributionRepo(t) + t.Setenv(remote.CheckpointTokenEnvVar, "") + t.Setenv(settings.EnvCheckpointsPrimary, "git-refs") + + missing := &attributionCheckpointReaderStub{} // sentinel not-found + oldOpen := openAttributionStore + openAttributionStore = func(context.Context) (attributionCheckpointReader, *git.Repository, error) { + return missing, nil, nil + } + t.Cleanup(func() { openAttributionStore = oldOpen }) + + resolver := newStubAttributionResolver(missing) + resolver.fetchOnMiss = true + ctx := resolver.readCheckpointContext(checkpointid.MustCheckpointID("cab2c3d4e5f0"), "auth.py") + + require.True(t, ctx.MetadataMissing) + require.NotContains(t, ctx.MetadataMissingReason, "remote refresh failed", + "a failing v1 fetch must be soft on a refs-primary repo") + require.Contains(t, ctx.MetadataMissingReason, "per-checkpoint refs") +} + +// Unreadable checkpoints config: neither backend's claim is supported, so a +// sentinel miss after a successful refresh gets generic neutral wording — not +// the strong git-branch claim (the failure direction must never overclaim) and +// not the refs-specific sentence. +func TestUnknownBackendConfigFailsToNeutralWording(t *testing.T) { + repoRoot := newAttributionRepo(t) + t.Setenv(settings.EnvCheckpointsPrimary, "") + require.NoError(t, os.MkdirAll(filepath.Join(repoRoot, ".entire"), 0o755)) + // Present-but-invalid checkpoints block (missing required primary.type): + // LoadCheckpointsConfig errors, leaving the backend unknown. + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, ".entire", "settings.json"), + []byte(`{"enabled":true,"checkpoints":{"primary":{}}}`), 0o644)) + + missing := &attributionCheckpointReaderStub{} // sentinel not-found + overrideAttributionRefresh(t, + func(context.Context) error { return nil }, + func(context.Context) (attributionCheckpointReader, *git.Repository, error) { + return missing, nil, nil + }) + + resolver := newStubAttributionResolver(missing) + resolver.fetchOnMiss = true + ctx := resolver.readCheckpointContext(checkpointid.MustCheckpointID("cbb2c3d4e5f0"), "auth.py") + + require.True(t, ctx.MetadataMissing) + require.NotContains(t, ctx.MetadataMissingReason, "does not contain it", + "an unreadable config must not select the strongest claim") + require.NotContains(t, ctx.MetadataMissingReason, "per-checkpoint refs", + "an unreadable config supports the refs wording no better") + require.Contains(t, ctx.MetadataMissingReason, "still unavailable") +} + // A refresh that succeeds while the summary read keeps failing for a // NON-absence reason (storage error, cancellation) proves nothing about the // remote: the reason must not claim "not pushed". From bf75bc71ead4997361e550e9cdb5a171b2948300 Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Thu, 2 Jul 2026 14:24:31 -0400 Subject: [PATCH 14/19] fix(attribution): restore branch scope; honest soft-refresh wording (audit round 13) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The round-12 commit accidentally swept in-progress why-TUI wiring into this branch while its defining file stayed untracked — the committed tree did not compile (undefined: runWhyTUI / whyMarkerLegend); local builds passed only via the untracked working-tree file. The TUI belongs to its own branch: the --tui flag, dispatch, repo-name helper, and legend line are removed here. Also, softened refreshes now stay honest: the soften decision moved out of the fetch seam into refreshMetadataStore, deciding via the SAME memoized backend gate missReason uses (one config resolution — no TOCTOU between the soften and evidence gates), and a soft refresh records refreshSoft so every wording says 'a metadata refresh was attempted (the v1 branch could not be fetched...)' instead of claiming the branch was refreshed. Pinned in the refs-primary soft-fetch test. --- cmd/entire/cli/attribution.go | 108 +++++++-------------- cmd/entire/cli/attribution_refresh_test.go | 4 + 2 files changed, 41 insertions(+), 71 deletions(-) diff --git a/cmd/entire/cli/attribution.go b/cmd/entire/cli/attribution.go index 24da834c54..202384babc 100644 --- a/cmd/entire/cli/attribution.go +++ b/cmd/entire/cli/attribution.go @@ -21,7 +21,6 @@ import ( "github.com/entireio/cli/cmd/entire/cli/checkpoint" "github.com/entireio/cli/cmd/entire/cli/checkpoint/id" "github.com/entireio/cli/cmd/entire/cli/checkpoint/remote" - "github.com/entireio/cli/cmd/entire/cli/interactive" "github.com/entireio/cli/cmd/entire/cli/logging" "github.com/entireio/cli/cmd/entire/cli/paths" "github.com/entireio/cli/cmd/entire/cli/settings" @@ -159,6 +158,12 @@ type attributionResolver struct { freshRepo *git.Repository // owned by the resolver; closed in Close storeGeneration int // bumped when a refresh swaps r.store + // refreshSoft records a refs-primary refresh whose v1-branch fetch failed + // but proceeded (per-checkpoint refs are fetched on demand): the wording + // must then say the refresh was attempted, not that the branch was + // refreshed. + refreshSoft bool + // Memoized backend topology (see primaryBackendIsRefs). refsPrimaryKnown bool refsPrimary bool @@ -197,7 +202,6 @@ func newBlameCmd() *cobra.Command { func newWhyCmd() *cobra.Command { var jsonFlag bool var lineFlag string - var tuiFlag bool cmd := &cobra.Command{ Use: "why [:line]", @@ -206,20 +210,18 @@ func newWhyCmd() *cobra.Command { // --help` keep working normally. Hidden: true, Short: "Show why a line exists", - Long: "Explain the commit, checkpoint, prompt, and session behind a file or line.\n\nTarget a specific line with :12 or the --line flag.\n\nUse --tui to browse the file interactively (TTY only; non-interactive runs fall back to the plain output).", + Long: "Explain the commit, checkpoint, prompt, and session behind a file or line.\n\nTarget a specific line with :12 or the --line flag.", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { return runAttributionWhy(cmd.Context(), cmd.OutOrStdout(), args[0], attributionWhyOptions{ LineFlag: lineFlag, JSON: jsonFlag, - TUI: tuiFlag, }) }, } cmd.Flags().StringVar(&lineFlag, "line", "", "Explain a specific line, for example 12 (same as :12)") cmd.Flags().BoolVar(&jsonFlag, "json", false, "Output explanation as JSON") - cmd.Flags().BoolVar(&tuiFlag, "tui", false, "Browse line attribution in an interactive viewer (TTY only)") return cmd } @@ -232,7 +234,6 @@ type attributionBlameOptions struct { type attributionWhyOptions struct { LineFlag string JSON bool - TUI bool } func runAttributionBlame(ctx context.Context, w io.Writer, file string, opts attributionBlameOptions) error { @@ -293,18 +294,6 @@ func runAttributionWhy(ctx context.Context, w io.Writer, target string, opts att return err } - // Interactive browsing is strictly opt-in AND TTY-gated (agent-safe CLI - // fallbacks): a non-interactive run with --tui falls through to the same - // deterministic plain/JSON output below, which carries the full - // information. --json wins over --tui so scripted callers never block. - if opts.TUI && !opts.JSON && interactive.IsTerminalWriter(w) && !IsAccessibleMode() { - startLine := 0 - if hasLine { - startLine = line - } - return runWhyTUI(result, attributionRepoFullName(ctx), shouldUseColor(w), startLine) - } - if !hasLine { if opts.JSON { return writeJSON(w, result) @@ -744,40 +733,16 @@ var fetchAttributionMetadata = func(ctx context.Context) error { return errors.New("checkpoint remote fetch failed: checkpoint_remote is configured but its URL could not be derived (resolution fell back to origin)") } if err := strategy.FetchMetadataBranch(ctx, url); err != nil { - if attributionRefsPrimarySoftensBranchFetch(ctx, err) { - return nil - } return fmt.Errorf("checkpoint remote fetch failed: %w", err) } return nil } if err := FetchMetadataBranch(ctx); err != nil { - if attributionRefsPrimarySoftensBranchFetch(ctx, err) { - return nil - } return fmt.Errorf("origin fetch failed: %w", err) } return nil } -// attributionRefsPrimarySoftensBranchFetch reports whether a failed v1-branch -// fetch should be treated as a soft outcome: on a git-refs-primary repo the -// branch is not the backend's primary mechanism (a refs-only remote may not -// carry it at all), and the reopened store's per-checkpoint RefFetcher does the -// real work — surfacing the branch failure would pin every miss in the run to -// "remote refresh failed: … entire/checkpoints/v1 …" plus a suggestion that -// cannot succeed. Only a KNOWN refs-primary topology softens the failure. -func attributionRefsPrimarySoftensBranchFetch(ctx context.Context, fetchErr error) bool { - cfg, cfgErr := settings.LoadCheckpointsConfig(ctx) - if cfgErr != nil || !checkpoint.PrimaryIsRefs(cfg) { - return false - } - logging.Debug(logging.WithComponent(ctx, "attribution"), - "v1 branch fetch failed on a git-refs-primary repo; treating as soft (per-checkpoint refs are fetched on demand)", - slog.String("error", fetchErr.Error())) - return true -} - // openAttributionStore opens a fresh checkpoint store over a freshly-opened // repo. After a fetch, the resolver's original repo handle can't see the new // packfiles (go-git's storer caches), so post-refresh reads need a new handle. @@ -834,10 +799,22 @@ func (r *attributionResolver) refreshMetadataStore() error { logCtx := logging.WithComponent(r.ctx, "attribution") if err := fetchAttributionMetadata(r.ctx); err != nil { - r.refreshErr = err - logging.Debug(logCtx, "checkpoint metadata remote refresh failed", - slog.String("error", err.Error())) - return r.refreshErr + // On a KNOWN git-refs-primary repo a failing v1-branch fetch is a soft + // outcome: the branch is not that backend's primary mechanism (a + // refs-only remote may not carry it), and the reopened store's + // per-checkpoint RefFetcher does the real work. Deciding via the same + // memoized gate missReason uses keeps the soften decision and the + // evidence wording on ONE config resolution (no TOCTOU between them). + if isRefs, known := r.primaryBackendIsRefs(); known && isRefs { + r.refreshSoft = true + logging.Debug(logCtx, "v1 branch fetch failed on a git-refs-primary repo; proceeding (per-checkpoint refs are fetched on demand)", + slog.String("error", err.Error())) + } else { + r.refreshErr = err + logging.Debug(logCtx, "checkpoint metadata remote refresh failed", + slog.String("error", err.Error())) + return r.refreshErr + } } store, repo, err := openAttributionStore(r.ctx) if err != nil { @@ -876,7 +853,7 @@ func (r *attributionResolver) missReason(checkpointID, base string, cause error, case r.refreshAttempted && summaryMissing && errors.Is(cause, checkpoint.ErrCheckpointNotFound) && missBackendIsRefs(r): // git-refs primary: the v1-branch refresh proves nothing about // per-checkpoint refs, so make no claim about what the remote holds. - return stringutil.CollapseWhitespace(reason + ". The metadata branch was refreshed, but this repo's primary checkpoint backend stores per-checkpoint refs, which that refresh may not cover — the checkpoint may not have been pushed yet, or its ref could not be fetched; attribution stays trailer-level.") + return stringutil.CollapseWhitespace(fmt.Sprintf("%s. %s, and this repo's primary checkpoint backend stores per-checkpoint refs, which it may not cover — the checkpoint may not have been pushed yet, or its ref could not be fetched; attribution stays trailer-level.", reason, r.refreshDescription())) case r.refreshAttempted && summaryMissing && errors.Is(cause, checkpoint.ErrCheckpointNotFound) && missBackendIsBranch(r): // Refresh succeeded and the refreshed store reports not-found: the // remote's metadata branch genuinely doesn't have it. @@ -884,19 +861,19 @@ func (r *attributionResolver) missReason(checkpointID, base string, cause error, case r.refreshAttempted && summaryMissing && errors.Is(cause, checkpoint.ErrCheckpointNotFound): // Backend unknown (config unreadable): neither the branch claim nor // the refs wording is supported — generic neutral text. - return stringutil.CollapseWhitespace(reason + ". The metadata branch was refreshed from the remote, but the checkpoint is still unavailable; attribution stays trailer-level.") + return stringutil.CollapseWhitespace(fmt.Sprintf("%s. %s, but the checkpoint is still unavailable; attribution stays trailer-level.", reason, r.refreshDescription())) case r.refreshAttempted && summaryMissing: // Refresh succeeded but the summary read failed for a reason other // than absence (storage error, cancellation): make no claim about // what the remote contains. The cause is already embedded above; no // log pointer — this path only emits debug-level entries. - return stringutil.CollapseWhitespace(reason + ". The metadata branch was refreshed from the remote, but reading the checkpoint still failed.") + return stringutil.CollapseWhitespace(fmt.Sprintf("%s. %s, but reading the checkpoint still failed.", reason, r.refreshDescription())) case r.refreshAttempted && retryReadErr != nil: // Refresh succeeded but RE-reading this checkpoint from the refreshed // store failed for a non-absence reason: the refreshed store was never // successfully consulted, so make no remote claim — surface the retry // error instead. - return stringutil.CollapseWhitespace(fmt.Sprintf("%s. The metadata branch was refreshed from the remote, but re-reading the checkpoint from the refreshed store failed (%v); attribution stays trailer-level.", reason, retryReadErr)) + return stringutil.CollapseWhitespace(fmt.Sprintf("%s. %s, but re-reading the checkpoint from the refreshed store failed (%v); attribution stays trailer-level.", reason, r.refreshDescription(), retryReadErr)) case r.refreshAttempted && remoteLacksCheckpoint && missBackendIsBranch(r): // The summary read locally, but a successful refresh proved the // remote lacks the checkpoint entirely: this is an unpushed @@ -1259,12 +1236,6 @@ func attributionLineMarker(line attributionLine) string { // renderAttributionMarkerLegend prints a one-line legend explaining the blame // markers, but only for the markers actually present in the table. func renderAttributionMarkerLegend(w io.Writer, sty statusStyles, lines []attributionLine) { - // Always explain the authorship tags — insiders found [MX] and [HU] - // confusing without it. Wording is deliberate (see whyMarkerLegend): - // [AI] means fully agent-authored checkpoint work, [MX] means agent work - // with human edits mixed in at commit, [HU] means no agent checkpoint. - fmt.Fprintf(w, " %s\n", sty.render(sty.dim, whyMarkerLegend)) - approximate, ambiguous := false, false for _, line := range lines { switch attributionLineMarker(line) { @@ -1723,21 +1694,6 @@ func shortCheckpointSession(line attributionLine) string { return line.CheckpointID + "/" + shortSessionID(line.SessionID) } -// attributionRepoFullName derives "owner/repo" from the origin remote for -// entire.io hyperlinks in the why TUI. Best-effort decoration: any failure -// (no origin, unparseable URL) returns "" and links are simply omitted. -func attributionRepoFullName(ctx context.Context) string { - originURL, err := remote.GetRemoteURL(ctx, "origin") - if err != nil || originURL == "" { - return "" - } - info, err := remote.ParseURL(originURL) - if err != nil || info.Owner == "" || info.Repo == "" { - return "" - } - return info.Owner + "/" + info.Repo -} - func shortSessionID(sessionID string) string { if len(sessionID) <= 8 { return sessionID @@ -1792,6 +1748,16 @@ func writeJSON(w io.Writer, value any) error { return nil } +// refreshDescription words what the refresh actually did: a soft refresh +// (refs-primary, v1-branch fetch failed but per-checkpoint refs fetch on +// demand) must not claim the branch was refreshed. +func (r *attributionResolver) refreshDescription() string { + if r.refreshSoft { + return "A metadata refresh was attempted (the v1 branch could not be fetched; this backend fetches per-checkpoint refs on demand)" + } + return "The metadata branch was refreshed from the remote" +} + // missBackendIsBranch / missBackendIsRefs are the evidence gates for // missReason: strong git-branch claims require a KNOWN git-branch backend, and // refs-specific wording requires a KNOWN git-refs backend. An unreadable diff --git a/cmd/entire/cli/attribution_refresh_test.go b/cmd/entire/cli/attribution_refresh_test.go index 5954bf3aef..9c21949d5a 100644 --- a/cmd/entire/cli/attribution_refresh_test.go +++ b/cmd/entire/cli/attribution_refresh_test.go @@ -222,6 +222,10 @@ func TestRefsPrimarySoftensFailingBranchFetch(t *testing.T) { require.NotContains(t, ctx.MetadataMissingReason, "remote refresh failed", "a failing v1 fetch must be soft on a refs-primary repo") require.Contains(t, ctx.MetadataMissingReason, "per-checkpoint refs") + // Honesty: the branch fetch FAILED, so the wording must say the refresh + // was attempted — not claim the branch was refreshed. + require.NotContains(t, ctx.MetadataMissingReason, "was refreshed") + require.Contains(t, ctx.MetadataMissingReason, "refresh was attempted") } // Unreadable checkpoints config: neither backend's claim is supported, so a From 61cef5f3d2ae901807e00883e24231d723665a7d Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Thu, 2 Jul 2026 14:46:45 -0400 Subject: [PATCH 15/19] fix(attribution): soften only genuine branch-fetch failures; honest catch-all (audit round 14) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 13's move of the soften decision into refreshMetadataStore accidentally widened its scope to EVERY refresh error: on a refs-primary repo, settings-read failures, URL-derivation failures, and the authority-guard rejection (the token-path evasion this branch hardened against) were silently softened behind a false 'the v1 branch could not be fetched' description. A sentinel now marks the genuine v1-branch fetch step; only errors carrying it are softened — config and policy failures stay hard on every backend, pinned by a refs-primary authority-guard test. The sessions-miss catch-all also now words a soft refresh honestly ('refresh was attempted', never 'after refreshing'), pinned by a soft-refresh sessions-miss test on the real fetch seam. --- cmd/entire/cli/attribution.go | 22 ++++++-- cmd/entire/cli/attribution_refresh_test.go | 61 +++++++++++++++++++++- 2 files changed, 77 insertions(+), 6 deletions(-) diff --git a/cmd/entire/cli/attribution.go b/cmd/entire/cli/attribution.go index 202384babc..f788c43a6a 100644 --- a/cmd/entire/cli/attribution.go +++ b/cmd/entire/cli/attribution.go @@ -733,16 +733,23 @@ var fetchAttributionMetadata = func(ctx context.Context) error { return errors.New("checkpoint remote fetch failed: checkpoint_remote is configured but its URL could not be derived (resolution fell back to origin)") } if err := strategy.FetchMetadataBranch(ctx, url); err != nil { - return fmt.Errorf("checkpoint remote fetch failed: %w", err) + return fmt.Errorf("checkpoint remote fetch failed: %w: %w", errAttributionBranchFetch, err) } return nil } if err := FetchMetadataBranch(ctx); err != nil { - return fmt.Errorf("origin fetch failed: %w", err) + return fmt.Errorf("origin fetch failed: %w: %w", errAttributionBranchFetch, err) } return nil } +// errAttributionBranchFetch marks a failure of the ACTUAL v1-branch fetch step +// (as opposed to settings-read, URL-derivation, or authority-guard failures, +// which must stay hard). Only this class is softened on a refs-primary repo: +// the branch is not that backend's primary mechanism, but an unreadable config +// or an unverifiable checkpoint remote is a real failure on any backend. +var errAttributionBranchFetch = errors.New("v1 branch fetch failed") + // openAttributionStore opens a fresh checkpoint store over a freshly-opened // repo. After a fetch, the resolver's original repo handle can't see the new // packfiles (go-git's storer caches), so post-refresh reads need a new handle. @@ -805,7 +812,12 @@ func (r *attributionResolver) refreshMetadataStore() error { // per-checkpoint RefFetcher does the real work. Deciding via the same // memoized gate missReason uses keeps the soften decision and the // evidence wording on ONE config resolution (no TOCTOU between them). - if isRefs, known := r.primaryBackendIsRefs(); known && isRefs { + // ONLY the genuine branch-fetch step is softened: settings-read, + // URL-derivation, and authority-guard failures stay hard on every + // backend (softening them would mask real config/policy failures + // behind a false "the v1 branch could not be fetched" description). + isRefs, known := r.primaryBackendIsRefs() + if errors.Is(err, errAttributionBranchFetch) && known && isRefs { r.refreshSoft = true logging.Debug(logCtx, "v1 branch fetch failed on a git-refs-primary repo; proceeding (per-checkpoint refs are fetched on demand)", slog.String("error", err.Error())) @@ -887,8 +899,8 @@ func (r *attributionResolver) missReason(checkpointID, base string, cause error, return stringutil.CollapseWhitespace(reason + ". They are still unavailable after refreshing from the remote — the metadata branch may have been pushed without them; attribution stays trailer-level.") case r.refreshAttempted: // git-refs primary or unknown backend: no speculation about the v1 - // branch's contents. - return stringutil.CollapseWhitespace(reason + ". They are still unavailable after refreshing; attribution stays trailer-level.") + // branch's contents; refreshDescription keeps a soft refresh honest. + return stringutil.CollapseWhitespace(fmt.Sprintf("%s. %s, but they are still unavailable; attribution stays trailer-level.", reason, r.refreshDescription())) } suggestion, isCommand := suggestCheckpointFetchCommand(r.ctx) diff --git a/cmd/entire/cli/attribution_refresh_test.go b/cmd/entire/cli/attribution_refresh_test.go index 9c21949d5a..7df930e621 100644 --- a/cmd/entire/cli/attribution_refresh_test.go +++ b/cmd/entire/cli/attribution_refresh_test.go @@ -191,11 +191,70 @@ func TestReadCheckpointContextRefsBackendWorseRetryStaysNeutral(t *testing.T) { ctx := resolver.readCheckpointContext(checkpointid.MustCheckpointID("fbb2c3d4e5f6"), "auth.py") require.True(t, ctx.MetadataMissing) - require.Contains(t, ctx.MetadataMissingReason, "still unavailable after refreshing") + require.Contains(t, ctx.MetadataMissingReason, "still unavailable") require.NotContains(t, ctx.MetadataMissingReason, "not on the remote", "the branch refresh cannot prove per-checkpoint-ref absence") } +// Refs-primary must NOT soften non-fetch failures: an authority-guard failure +// (checkpoint_remote URL underivable) is a real policy failure on every +// backend — softening it would mask it behind a false "the v1 branch could not +// be fetched" description while per-checkpoint refs quietly fetch from origin. +func TestRefsPrimaryDoesNotSoftenAuthorityGuardFailure(t *testing.T) { + repoRoot := newAttributionRepo(t) + runAttributionGit(t, repoRoot, "remote", "add", "origin", "git@github.com:org/app.git") + require.NoError(t, os.MkdirAll(filepath.Join(repoRoot, ".entire"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, ".entire", "settings.json"), + []byte(`{"enabled":true,"strategy_options":{"checkpoint_remote":{"provider":"bitbucket","repo":"org/checkpoints"}}}`), 0o644)) + t.Setenv(remote.CheckpointTokenEnvVar, "test-token") + t.Setenv(settings.EnvCheckpointsPrimary, "git-refs") + + resolver := newStubAttributionResolver(&attributionCheckpointReaderStub{readErr: errors.New("object not found")}) + resolver.fetchOnMiss = true + ctx := resolver.readCheckpointContext(checkpointid.MustCheckpointID("ddb2c3d4e5f1"), "auth.py") + + require.True(t, ctx.MetadataMissing) + require.Contains(t, ctx.MetadataMissingReason, "remote refresh failed", + "authority-guard failures must stay hard even on refs-primary") + require.Contains(t, ctx.MetadataMissingReason, "fell back to origin") + require.NotContains(t, ctx.MetadataMissingReason, "refresh was attempted", + "must not describe a policy failure as a soft branch-fetch miss") +} + +// A soft refresh (v1 fetch genuinely failed on refs-primary) must stay honest +// in the sessions-miss catch-all too: "refresh was attempted", never "after +// refreshing". +func TestRefsPrimarySoftSessionsMissStaysHonest(t *testing.T) { + newAttributionRepo(t) + t.Setenv(remote.CheckpointTokenEnvVar, "") + t.Setenv(settings.EnvCheckpointsPrimary, "git-refs") + + // Summary readable, sessions unreadable — on BOTH stores. + broken := &attributionCheckpointReaderStub{ + summary: &checkpoint.CheckpointSummary{ + Sessions: []checkpoint.SessionFilePaths{{Metadata: "metadata.json"}}, + }, + sessionErr: errors.New("object not found"), + } + // Real fetch seam (no origin -> genuine branch-fetch failure -> soft); + // override only the reopen. + oldOpen := openAttributionStore + openAttributionStore = func(context.Context) (attributionCheckpointReader, *git.Repository, error) { + return broken, nil, nil + } + t.Cleanup(func() { openAttributionStore = oldOpen }) + + resolver := newStubAttributionResolver(broken) + resolver.fetchOnMiss = true + ctx := resolver.readCheckpointContext(checkpointid.MustCheckpointID("deb2c3d4e5f1"), "auth.py") + + require.True(t, ctx.MetadataMissing) + require.Contains(t, ctx.MetadataMissingReason, "refresh was attempted", + "the v1 fetch failed; the wording must say attempted") + require.NotContains(t, ctx.MetadataMissingReason, "was refreshed") + require.NotContains(t, ctx.MetadataMissingReason, "after refreshing") +} + // refs-primary + failing v1-branch fetch: the branch is not that backend's // primary mechanism, so the failure is soft — the refresh proceeds (reopened // store's RefFetcher does the real work) and misses get the refs wording, not From 0211097008d46f93fca43648a08c673b81a9b7e7 Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Thu, 2 Jul 2026 15:48:53 -0400 Subject: [PATCH 16/19] =?UTF-8?q?fix(attribution):=20exhaustive-audit=20cl?= =?UTF-8?q?eanup=20=E2=80=94=20soft-refresh=20honesty,=20refs-aware=20sugg?= =?UTF-8?q?estion,=20env-hermetic=20tests=20(audit=20round=2015)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An exhaustive 8-lens, loop-until-dry audit (full missReason state-machine enumeration, adversarial scenario construction, both-backend test matrix) confirmed the following; all fixed: - Soft-refresh honesty completed: the rejected-retry wording no longer calls the store 'refreshed' in the same sentence that admits the v1 branch was not fetched; the reopen debug log is soft-aware; and a soft refresh whose store reopen ALSO fails keeps the swallowed fetch error as diagnostic context. - Refs-aware recovery: a refs-primary no-refresh miss now suggests fetching the per-checkpoint refs namespace instead of the v1 branch that cannot heal it (suggestCheckpointFetchCommandForRefspec; the v1 wording is unchanged for the git-branch backend and for attach). - Env hermeticity closed: newAttributionRepo pins the git-branch backend by default (tests wanting git-refs override after the call), the two remaining backend-sensitive tests pin explicitly, and the blame no-fetch contract is now non-vacuous (seam traps fail the test if fetchOnMiss=false ever fetches). - suggestCheckpointFetchCommand's four outcomes pinned directly (derived command / origin command / unreadable-settings prose / underivable-URL prose); overstated 'exactly once' comment corrected; misleading pin-rationale comment fixed. Verified under hostile environments: full attribution suite green with ENTIRE_CHECKPOINTS_PRIMARY=git-refs and with a leaked ENTIRE_CHECKPOINT_TOKEN. --- cmd/entire/cli/attach.go | 15 ++++- cmd/entire/cli/attribution.go | 37 +++++++--- cmd/entire/cli/attribution_refresh_test.go | 78 +++++++++++++++++++++- cmd/entire/cli/attribution_test.go | 7 ++ 4 files changed, 123 insertions(+), 14 deletions(-) diff --git a/cmd/entire/cli/attach.go b/cmd/entire/cli/attach.go index f33b79b8c5..133b5a9f78 100644 --- a/cmd/entire/cli/attach.go +++ b/cmd/entire/cli/attach.go @@ -599,7 +599,20 @@ func checkpointPresentLocally(ctx context.Context, repo *git.Repository, refs cp // "fetch origin" is not safe guidance (remote.Configured would collapse that // case to "not configured"). func suggestCheckpointFetchCommand(ctx context.Context) (suggestion string, isCommand bool) { - ref := "entire/checkpoints/v1:entire/checkpoints/v1" + return suggestCheckpointFetchCommandForRefspec(ctx, checkpointBranchRefspec) +} + +// Refspecs for the two checkpoint storage topologies: the v1 metadata branch +// (git-branch backend, and what attach/resume consume) and the per-checkpoint +// refs namespace (git-refs backend, where a v1-branch fetch cannot heal a miss). +const ( + checkpointBranchRefspec = "entire/checkpoints/v1:entire/checkpoints/v1" + checkpointRefsRefspec = "'+refs/entire/checkpoints/*:refs/entire/checkpoints/*'" +) + +// suggestCheckpointFetchCommandForRefspec is suggestCheckpointFetchCommand with +// the refspec chosen by the caller (branch vs per-checkpoint refs). +func suggestCheckpointFetchCommandForRefspec(ctx context.Context, ref string) (suggestion string, isCommand bool) { s, err := settings.Load(ctx) if err != nil { return ".entire/settings.json could not be read — fix it first; the correct fetch remote depends on its checkpoint_remote setting", false diff --git a/cmd/entire/cli/attribution.go b/cmd/entire/cli/attribution.go index f788c43a6a..5f1da4b67f 100644 --- a/cmd/entire/cli/attribution.go +++ b/cmd/entire/cli/attribution.go @@ -706,13 +706,13 @@ var fetchAttributionMetadata = func(ctx context.Context) error { // data: do not mask its failure with an origin fallback that may hold a // stale copy (or no metadata branch at all) — a later miss would then // claim "not pushed" about a remote we never reached. - // Resolve the authority exactly once, then fetch the URL that was - // verified. Independently re-resolving inside the fetch helper (which - // re-reads settings and git remotes) opened a TOCTOU window where a - // transient divergence could route the fetch to origin while attribution - // recorded an authoritative checkpoint-remote refresh. A settings-load - // failure is likewise a refresh failure — not "no checkpoint remote", - // which would silently promote origin to the evidence source. + // Resolve the authority up front, then fetch the URL that was verified. + // (FetchURLWithSource internally re-reads settings; a divergence between + // the two loads fails closed — the explicit source bool below rejects any + // resolution that did not come from the configured checkpoint remote.) + // A settings-load failure is likewise a refresh failure — not "no + // checkpoint remote", which would silently promote origin to the evidence + // source. s, err := settings.Load(ctx) if err != nil { return fmt.Errorf("checkpoint remote fetch failed: read settings: %w", err) @@ -805,6 +805,7 @@ func (r *attributionResolver) refreshMetadataStore() error { r.refreshAttempted = true logCtx := logging.WithComponent(r.ctx, "attribution") + var softFetchErr error if err := fetchAttributionMetadata(r.ctx); err != nil { // On a KNOWN git-refs-primary repo a failing v1-branch fetch is a soft // outcome: the branch is not that backend's primary mechanism (a @@ -819,6 +820,7 @@ func (r *attributionResolver) refreshMetadataStore() error { isRefs, known := r.primaryBackendIsRefs() if errors.Is(err, errAttributionBranchFetch) && known && isRefs { r.refreshSoft = true + softFetchErr = err logging.Debug(logCtx, "v1 branch fetch failed on a git-refs-primary repo; proceeding (per-checkpoint refs are fetched on demand)", slog.String("error", err.Error())) } else { @@ -830,6 +832,11 @@ func (r *attributionResolver) refreshMetadataStore() error { } store, repo, err := openAttributionStore(r.ctx) if err != nil { + // A soft refresh's swallowed fetch error is still diagnostic context + // when the reopen ALSO fails — keep both. + if softFetchErr != nil { + err = fmt.Errorf("%w (v1 branch fetch had also failed: %w)", err, softFetchErr) + } r.refreshErr = err logging.Debug(logCtx, "checkpoint store reopen after refresh failed", slog.String("error", err.Error())) @@ -838,7 +845,11 @@ func (r *attributionResolver) refreshMetadataStore() error { r.freshRepo = repo r.store = store r.storeGeneration++ - logging.Debug(logCtx, "checkpoint metadata refreshed from remote") + if r.refreshSoft { + logging.Debug(logCtx, "checkpoint store reopened after soft refresh (v1 branch not fetched; per-checkpoint refs fetch on demand)") + } else { + logging.Debug(logCtx, "checkpoint metadata refreshed from remote") + } return nil } @@ -885,7 +896,7 @@ func (r *attributionResolver) missReason(checkpointID, base string, cause error, // store failed for a non-absence reason: the refreshed store was never // successfully consulted, so make no remote claim — surface the retry // error instead. - return stringutil.CollapseWhitespace(fmt.Sprintf("%s. %s, but re-reading the checkpoint from the refreshed store failed (%v); attribution stays trailer-level.", reason, r.refreshDescription(), retryReadErr)) + return stringutil.CollapseWhitespace(fmt.Sprintf("%s. %s, but re-reading the checkpoint after the refresh attempt failed (%v); attribution stays trailer-level.", reason, r.refreshDescription(), retryReadErr)) case r.refreshAttempted && remoteLacksCheckpoint && missBackendIsBranch(r): // The summary read locally, but a successful refresh proved the // remote lacks the checkpoint entirely: this is an unpushed @@ -903,7 +914,13 @@ func (r *attributionResolver) missReason(checkpointID, base string, cause error, return stringutil.CollapseWhitespace(fmt.Sprintf("%s. %s, but they are still unavailable; attribution stays trailer-level.", reason, r.refreshDescription())) } - suggestion, isCommand := suggestCheckpointFetchCommand(r.ctx) + refspec := checkpointBranchRefspec + if isRefs, known := r.primaryBackendIsRefs(); known && isRefs { + // A v1-branch fetch cannot heal a refs-backend miss — suggest the + // per-checkpoint refs fetch instead. + refspec = checkpointRefsRefspec + } + suggestion, isCommand := suggestCheckpointFetchCommandForRefspec(r.ctx, refspec) if isCommand { suggestion = "Run: " + suggestion } diff --git a/cmd/entire/cli/attribution_refresh_test.go b/cmd/entire/cli/attribution_refresh_test.go index 7df930e621..56a0d99c09 100644 --- a/cmd/entire/cli/attribution_refresh_test.go +++ b/cmd/entire/cli/attribution_refresh_test.go @@ -82,7 +82,14 @@ func TestReadCheckpointContextSessionsUnreadableGetsReason(t *testing.T) { } // fetchOnMiss=false (the blame path): no refresh, but the reason must still - // name the cause and the recovery command. + // name the cause and the recovery command. The seam trap makes the + // no-refresh contract non-vacuous. + overrideAttributionRefresh(t, + func(context.Context) error { t.Fatal("blame path must not fetch"); return nil }, + func(context.Context) (attributionCheckpointReader, *git.Repository, error) { + t.Fatal("blame path must not reopen the store") + return nil, nil, nil + }) ctx := newStubAttributionResolver(reader).readCheckpointContext(cpID, "auth.py") require.True(t, ctx.MetadataMissing) require.Contains(t, ctx.MetadataMissingReason, "session record") @@ -253,6 +260,67 @@ func TestRefsPrimarySoftSessionsMissStaysHonest(t *testing.T) { "the v1 fetch failed; the wording must say attempted") require.NotContains(t, ctx.MetadataMissingReason, "was refreshed") require.NotContains(t, ctx.MetadataMissingReason, "after refreshing") + require.NotContains(t, ctx.MetadataMissingReason, "refreshed store", + "a soft refresh reopened the store; it did not refresh it") +} + +// H: the suggestion tuple, pinned directly for all four outcomes. +func TestSuggestCheckpointFetchCommandOutcomes(t *testing.T) { + t.Run("derivable checkpoint remote is a command", func(t *testing.T) { + repoRoot := newAttributionRepo(t) + t.Setenv(remote.CheckpointTokenEnvVar, "") + runAttributionGit(t, repoRoot, "remote", "add", "origin", "git@github.com:org/app.git") + require.NoError(t, os.MkdirAll(filepath.Join(repoRoot, ".entire"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, ".entire", "settings.json"), + []byte(`{"enabled":true,"strategy_options":{"checkpoint_remote":{"provider":"github","repo":"org/checkpoints"}}}`), 0o644)) + suggestion, isCommand := suggestCheckpointFetchCommand(t.Context()) + require.True(t, isCommand) + require.Equal(t, "git fetch git@github.com:org/checkpoints.git entire/checkpoints/v1:entire/checkpoints/v1", suggestion) + }) + t.Run("no checkpoint remote is the origin command", func(t *testing.T) { + newAttributionRepo(t) + t.Setenv(remote.CheckpointTokenEnvVar, "") + suggestion, isCommand := suggestCheckpointFetchCommand(t.Context()) + require.True(t, isCommand) + require.Equal(t, "git fetch origin entire/checkpoints/v1:entire/checkpoints/v1", suggestion) + }) + t.Run("unreadable settings is prose", func(t *testing.T) { + repoRoot := newAttributionRepo(t) + require.NoError(t, os.MkdirAll(filepath.Join(repoRoot, ".entire"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, ".entire", "settings.json"), + []byte(`{not valid json`), 0o644)) + suggestion, isCommand := suggestCheckpointFetchCommand(t.Context()) + require.False(t, isCommand) + require.Contains(t, suggestion, "settings.json") + }) + t.Run("underivable checkpoint remote URL is prose", func(t *testing.T) { + repoRoot := newAttributionRepo(t) + t.Setenv(remote.CheckpointTokenEnvVar, "") + // checkpoint_remote configured, but no origin to derive protocol from. + require.NoError(t, os.MkdirAll(filepath.Join(repoRoot, ".entire"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(repoRoot, ".entire", "settings.json"), + []byte(`{"enabled":true,"strategy_options":{"checkpoint_remote":{"provider":"github","repo":"org/checkpoints"}}}`), 0o644)) + suggestion, isCommand := suggestCheckpointFetchCommand(t.Context()) + require.False(t, isCommand) + require.Contains(t, suggestion, "checkpoint_remote") + }) +} + +// F pin: a refs-primary no-refresh miss (blame path) must suggest the +// per-checkpoint refs fetch, not the v1 branch that cannot heal it. +func TestRefsPrimaryNoRefreshSuggestionUsesRefsRefspec(t *testing.T) { + newAttributionRepo(t) + t.Setenv(remote.CheckpointTokenEnvVar, "") + t.Setenv(settings.EnvCheckpointsPrimary, "git-refs") + + reader := &attributionCheckpointReaderStub{readErr: errors.New("object not found")} + ctx := newStubAttributionResolver(reader).readCheckpointContext(checkpointid.MustCheckpointID("dfb2c3d4e5f1"), "auth.py") + + require.True(t, ctx.MetadataMissing) + require.Contains(t, ctx.MetadataMissingReason, "refs/entire/checkpoints/*", + "refs-backend recovery must fetch the per-checkpoint refs namespace") + require.NotContains(t, ctx.MetadataMissingReason, "entire/checkpoints/v1:entire/checkpoints/v1", + "a v1-branch fetch cannot heal a refs-backend miss") } // refs-primary + failing v1-branch fetch: the branch is not that backend's @@ -432,7 +500,8 @@ func TestReadCheckpointContextKeepsLocalWhenRefreshReadsWorse(t *testing.T) { // neutral and surface the retry's error rather than speculating the metadata // branch "may have been pushed without" the session records. func TestReadCheckpointContextRejectedRetryNonAbsenceStaysNeutral(t *testing.T) { - // Pin the backend so the retryReadErr branch (not the refs branch) fires. + // Backend pinned for hermeticity only — the retryReadErr case fires + // before any backend gate. t.Setenv(settings.EnvCheckpointsPrimary, "git-branch") cpID := checkpointid.MustCheckpointID("fdb2c3d4e5f6") local := &attributionCheckpointReaderStub{ @@ -682,8 +751,11 @@ func TestRefreshCorruptSettingsSuggestionDoesNotSayFetchOrigin(t *testing.T) { func TestRefreshWithLocalBranchButNoRemoteReportsFetchFailure(t *testing.T) { repoRoot := newAttributionRepo(t) // Hermeticity: ensure a developer/CI ENTIRE_CHECKPOINT_TOKEN can't turn - // this into a network-dependent test. + // this into a network-dependent test, and pin the backend — the asserted + // hard-failure wording is the git-branch outcome (a refs-primary env + // would soften this exact fetch failure). t.Setenv(remote.CheckpointTokenEnvVar, "") + t.Setenv(settings.EnvCheckpointsPrimary, "git-branch") // A local entire/checkpoints/v1 exists (the old getMetadataTree fallback // would "succeed" from it); the queried checkpoint is not in it. writeAttributionCheckpoint(t, repoRoot, "99b2c3d4e5f6", checkpoint.WriteOptions{ diff --git a/cmd/entire/cli/attribution_test.go b/cmd/entire/cli/attribution_test.go index 120c06c4cf..d629459a1b 100644 --- a/cmd/entire/cli/attribution_test.go +++ b/cmd/entire/cli/attribution_test.go @@ -17,6 +17,7 @@ import ( "github.com/entireio/cli/cmd/entire/cli/checkpoint" checkpointid "github.com/entireio/cli/cmd/entire/cli/checkpoint/id" "github.com/entireio/cli/cmd/entire/cli/paths" + "github.com/entireio/cli/cmd/entire/cli/settings" "github.com/entireio/cli/cmd/entire/cli/testutil" "github.com/entireio/cli/cmd/entire/cli/trailers" "github.com/entireio/cli/redact" @@ -740,6 +741,12 @@ func newAttributionRepo(t *testing.T) string { repoRoot := t.TempDir() testutil.InitRepo(t, repoRoot) t.Chdir(repoRoot) + // Default every attribution test to the git-branch backend: these repos + // write checkpoints through the git-branch store, so an ambient + // ENTIRE_CHECKPOINTS_PRIMARY=git-refs would read them through the wrong + // backend. Tests that WANT git-refs override this after the call (their + // later t.Setenv wins). + t.Setenv(settings.EnvCheckpointsPrimary, "git-branch") paths.ClearWorktreeRootCache() t.Cleanup(paths.ClearWorktreeRootCache) From 9ef283498f5684617ecc0be24230ae3e10730eba Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Thu, 2 Jul 2026 15:51:54 -0400 Subject: [PATCH 17/19] feat(attribution): interactive why viewer + tag legend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an opt-in TUI for 'entire why ' and the readability fixes insiders asked for, built under the Agent-Safe CLI Fallbacks rules: - --tui opens a master-detail Bubble Tea viewer over the already-resolved attribution (no git/network I/O inside the TUI): file lines with [AI]/[MX]/[HU] tags on the left; the selected line's full explanation on the right (authorship sentence, commit, agent/model, session with an OSC 8 entire.io hyperlink when origin resolves, checkpoint, prompt with the session-level caveat, intent, metadata-missing reason, candidates, explain hint). n/p jump to the next/previous agent-attributed line; enter expands the full prompt; why :12 --tui opens positioned at line 12. - Agent-safe: the TUI is opt-in AND TTY-gated (IsTerminalWriter) and skipped in accessible mode; non-TTY --tui falls through to the identical plain output, and --json always wins — both pinned by tests against a non-TTY writer. Nothing is TUI-only. - Tag legend on blame, the why file view, and the TUI footer: [AI] all agent, [MX] agent+human mixed, [HU] no agent, [??] uncommitted — kept within the blame table's 80-column budget; full sentences in the detail views. Model-level TUI tests cover rendering, navigation, agent-line jumps, expand, missing-metadata handling, quit keys, empty files, and tiny windows. Paging is bound to pgup/pgdn because the shared keymap claims n/p. --- cmd/entire/cli/attribution.go | 40 +- cmd/entire/cli/attribution_tui.go | 545 +++++++++++++++++++++++++ cmd/entire/cli/attribution_tui_test.go | 280 +++++++++++++ 3 files changed, 864 insertions(+), 1 deletion(-) create mode 100644 cmd/entire/cli/attribution_tui.go create mode 100644 cmd/entire/cli/attribution_tui_test.go diff --git a/cmd/entire/cli/attribution.go b/cmd/entire/cli/attribution.go index 5f1da4b67f..d191020f02 100644 --- a/cmd/entire/cli/attribution.go +++ b/cmd/entire/cli/attribution.go @@ -21,6 +21,7 @@ import ( "github.com/entireio/cli/cmd/entire/cli/checkpoint" "github.com/entireio/cli/cmd/entire/cli/checkpoint/id" "github.com/entireio/cli/cmd/entire/cli/checkpoint/remote" + "github.com/entireio/cli/cmd/entire/cli/interactive" "github.com/entireio/cli/cmd/entire/cli/logging" "github.com/entireio/cli/cmd/entire/cli/paths" "github.com/entireio/cli/cmd/entire/cli/settings" @@ -202,6 +203,7 @@ func newBlameCmd() *cobra.Command { func newWhyCmd() *cobra.Command { var jsonFlag bool var lineFlag string + var tuiFlag bool cmd := &cobra.Command{ Use: "why [:line]", @@ -210,18 +212,20 @@ func newWhyCmd() *cobra.Command { // --help` keep working normally. Hidden: true, Short: "Show why a line exists", - Long: "Explain the commit, checkpoint, prompt, and session behind a file or line.\n\nTarget a specific line with :12 or the --line flag.", + Long: "Explain the commit, checkpoint, prompt, and session behind a file or line.\n\nTarget a specific line with :12 or the --line flag.\n\nUse --tui to browse the file interactively (TTY only; non-interactive runs fall back to the plain output).", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { return runAttributionWhy(cmd.Context(), cmd.OutOrStdout(), args[0], attributionWhyOptions{ LineFlag: lineFlag, JSON: jsonFlag, + TUI: tuiFlag, }) }, } cmd.Flags().StringVar(&lineFlag, "line", "", "Explain a specific line, for example 12 (same as :12)") cmd.Flags().BoolVar(&jsonFlag, "json", false, "Output explanation as JSON") + cmd.Flags().BoolVar(&tuiFlag, "tui", false, "Browse line attribution in an interactive viewer (TTY only)") return cmd } @@ -234,6 +238,7 @@ type attributionBlameOptions struct { type attributionWhyOptions struct { LineFlag string JSON bool + TUI bool } func runAttributionBlame(ctx context.Context, w io.Writer, file string, opts attributionBlameOptions) error { @@ -294,6 +299,18 @@ func runAttributionWhy(ctx context.Context, w io.Writer, target string, opts att return err } + // Interactive browsing is strictly opt-in AND TTY-gated (agent-safe CLI + // fallbacks): a non-interactive run with --tui falls through to the same + // deterministic plain/JSON output below, which carries the full + // information. --json wins over --tui so scripted callers never block. + if opts.TUI && !opts.JSON && interactive.IsTerminalWriter(w) && !IsAccessibleMode() { + startLine := 0 + if hasLine { + startLine = line + } + return runWhyTUI(result, attributionRepoFullName(ctx), shouldUseColor(w), startLine) + } + if !hasLine { if opts.JSON { return writeJSON(w, result) @@ -1265,6 +1282,12 @@ func attributionLineMarker(line attributionLine) string { // renderAttributionMarkerLegend prints a one-line legend explaining the blame // markers, but only for the markers actually present in the table. func renderAttributionMarkerLegend(w io.Writer, sty statusStyles, lines []attributionLine) { + // Always explain the authorship tags — insiders found [MX] and [HU] + // confusing without it. Wording is deliberate (see whyMarkerLegend): + // [AI] means fully agent-authored checkpoint work, [MX] means agent work + // with human edits mixed in at commit, [HU] means no agent checkpoint. + fmt.Fprintf(w, " %s\n", sty.render(sty.dim, whyMarkerLegend)) + approximate, ambiguous := false, false for _, line := range lines { switch attributionLineMarker(line) { @@ -1723,6 +1746,21 @@ func shortCheckpointSession(line attributionLine) string { return line.CheckpointID + "/" + shortSessionID(line.SessionID) } +// attributionRepoFullName derives "owner/repo" from the origin remote for +// entire.io hyperlinks in the why TUI. Best-effort decoration: any failure +// (no origin, unparseable URL) returns "" and links are simply omitted. +func attributionRepoFullName(ctx context.Context) string { + originURL, err := remote.GetRemoteURL(ctx, "origin") + if err != nil || originURL == "" { + return "" + } + info, err := remote.ParseURL(originURL) + if err != nil || info.Owner == "" || info.Repo == "" { + return "" + } + return info.Owner + "/" + info.Repo +} + func shortSessionID(sessionID string) string { if len(sessionID) <= 8 { return sessionID diff --git a/cmd/entire/cli/attribution_tui.go b/cmd/entire/cli/attribution_tui.go new file mode 100644 index 0000000000..440fb6838c --- /dev/null +++ b/cmd/entire/cli/attribution_tui.go @@ -0,0 +1,545 @@ +package cli + +import ( + "fmt" + "net/url" + "strconv" + "strings" + + "charm.land/bubbles/v2/key" + "charm.land/bubbles/v2/viewport" + tea "charm.land/bubbletea/v2" + "charm.land/lipgloss/v2" + + "github.com/entireio/cli/cmd/entire/cli/stringutil" +) + +// Layout budget for the why viewer. The header is a title line plus a blank +// line; the footer is a marker legend line plus a help line. The remaining rows +// are split between the file-line list (left) and the selected line's +// explanation (right), separated by a thin vertical rule. +const ( + whyHeaderHeight = 2 + whyFooterHeight = 2 + whyListMinWidth = 34 + whyListWidthRatio = 0.55 // list gets slightly more room than the detail + whyPaneGap = 3 // " │ " +) + +// whyMarkerLegend is the one-line explanation of the attribution tags shown in +// the footer and in the plain-text views. Wording is deliberate: [AI] means the +// commit's checkpointed work was fully agent-authored; [MX] means agent work +// with human edits mixed in at commit; [HU] means no agent checkpoint recorded. +// Kept within the blame table's 80-column budget (with its 2-space indent); the +// full sentences live in the why detail views. +const whyMarkerLegend = "[AI] all agent · [MX] agent+human mixed · [HU] no agent · [??] uncommitted" + +// whyTUIStyles holds the interactive viewer's palette. Empty styles render as +// plain text when color is off, which also keeps tests assertable. +type whyTUIStyles struct { + colorEnabled bool + + title lipgloss.Style + file lipgloss.Style + dim lipgloss.Style + selected lipgloss.Style + section lipgloss.Style + tagAI lipgloss.Style + tagMX lipgloss.Style + tagHU lipgloss.Style + warn lipgloss.Style + helpKey lipgloss.Style + helpDesc lipgloss.Style + sepBar lipgloss.Style +} + +func newWhyTUIStyles(useColor bool) whyTUIStyles { + s := whyTUIStyles{colorEnabled: useColor} + if !useColor { + return s + } + s.title = lipgloss.NewStyle().Bold(true) + s.file = lipgloss.NewStyle().Foreground(lipgloss.Color("#fb923c")).Bold(true) + s.dim = lipgloss.NewStyle().Foreground(lipgloss.Color("241")) + s.selected = lipgloss.NewStyle().Foreground(lipgloss.Color("#fb923c")).Bold(true) + s.section = lipgloss.NewStyle().Foreground(lipgloss.Color("#fb923c")).Bold(true) + s.tagAI = lipgloss.NewStyle().Foreground(lipgloss.Color("2")) + s.tagMX = lipgloss.NewStyle().Foreground(lipgloss.Color("3")) + s.tagHU = lipgloss.NewStyle().Foreground(lipgloss.Color("245")) + s.warn = lipgloss.NewStyle().Foreground(lipgloss.Color("3")) + s.helpKey = lipgloss.NewStyle().Foreground(lipgloss.Color("245")).Bold(true) + s.helpDesc = lipgloss.NewStyle().Foreground(lipgloss.Color("241")) + s.sepBar = lipgloss.NewStyle().Foreground(lipgloss.Color("8")) + return s +} + +func (s whyTUIStyles) render(style lipgloss.Style, text string) string { + if !s.colorEnabled { + return text + } + return style.Render(text) +} + +// link renders text with an OSC 8 hyperlink when styling is enabled; plain +// styled text otherwise, so dumb terminals and piped output are unaffected. +func (s whyTUIStyles) link(style lipgloss.Style, linkURL, text string) string { + if !s.colorEnabled || strings.TrimSpace(linkURL) == "" { + return s.render(style, text) + } + return style.Hyperlink(linkURL).Render(text) +} + +func (s whyTUIStyles) tagStyle(tag string) lipgloss.Style { + switch tag { + case "[AI]": + return s.tagAI + case "[MX]": + return s.tagMX + default: + return s.tagHU + } +} + +// whyTUIModel renders a master-detail view over a pre-resolved file +// attribution: the file's lines on the left (with attribution markers) and the +// selected line's full explanation on the right. All data is resolved before +// the program starts — no git or network I/O happens inside the TUI. +type whyTUIModel struct { + result *fileAttributionResult + styles whyTUIStyles + + // repoFullName ("owner/repo") enables entire.io session hyperlinks; empty + // disables them (links are best-effort decoration, never required). + repoFullName string + + cursor int + listTop int // first visible list row (manual scroll window) + expanded bool + width int + height int + ready bool + vp viewport.Model + statusMsg string +} + +func newWhyTUIModel(result *fileAttributionResult, repoFullName string, useColor bool, startLine int) whyTUIModel { + m := whyTUIModel{ + result: result, + styles: newWhyTUIStyles(useColor), + repoFullName: repoFullName, + } + if startLine > 0 { + for i := range result.Lines { + if result.Lines[i].LineNumber == startLine { + m.cursor = i + break + } + } + } + return m +} + +func runWhyTUI(result *fileAttributionResult, repoFullName string, useColor bool, startLine int) error { + p := tea.NewProgram(newWhyTUIModel(result, repoFullName, useColor, startLine)) + if _, err := p.Run(); err != nil { + return fmt.Errorf("why TUI: %w", err) + } + return nil +} + +func (m whyTUIModel) Init() tea.Cmd { return nil } + +func (m whyTUIModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { + switch msg := msg.(type) { + case tea.WindowSizeMsg: + m.width = msg.Width + m.height = msg.Height + m = m.layout() + return m, nil + case tea.KeyPressMsg: + return m.handleKey(msg) + } + if m.ready { + var cmd tea.Cmd + m.vp, cmd = m.vp.Update(msg) + return m, cmd + } + return m, nil +} + +func (m whyTUIModel) handleKey(msg tea.KeyPressMsg) (tea.Model, tea.Cmd) { + switch { + case key.Matches(msg, keys.Quit), key.Matches(msg, keys.Back): + return m, tea.Quit + case key.Matches(msg, keys.Up): + return m.moveCursor(m.cursor - 1), nil + case key.Matches(msg, keys.Down): + return m.moveCursor(m.cursor + 1), nil + // Paging is bound to pgup/pgdown explicitly: the shared keymap's + // NextPage/PrevPage claim n/p, which this viewer uses for the more useful + // next/previous agent-attributed line jumps. + case msg.String() == "pgup": + return m.moveCursor(m.cursor - m.bodyHeight()), nil + case msg.String() == "pgdown": + return m.moveCursor(m.cursor + m.bodyHeight()), nil + case key.Matches(msg, keys.Home): + return m.moveCursor(0), nil + case key.Matches(msg, keys.End): + return m.moveCursor(len(m.result.Lines) - 1), nil + case key.Matches(msg, keys.Confirm): + m.expanded = !m.expanded + return m.refreshDetail(), nil + case msg.String() == "n": + return m.jumpAgentLine(1), nil + case msg.String() == "p", msg.String() == "N": + return m.jumpAgentLine(-1), nil + } + if m.ready { + var cmd tea.Cmd + m.vp, cmd = m.vp.Update(msg) + return m, cmd + } + return m, nil +} + +func (m whyTUIModel) moveCursor(to int) whyTUIModel { + if len(m.result.Lines) == 0 { + return m + } + if to < 0 { + to = 0 + } + if to > len(m.result.Lines)-1 { + to = len(m.result.Lines) - 1 + } + if to == m.cursor { + return m + } + m.cursor = to + m.expanded = false + m = m.scrollListToCursor() + return m.refreshDetail() +} + +// scrollListToCursor adjusts the persisted list window so the cursor stays +// visible. Kept separate from rendering so the scroll state survives value- +// receiver renders. +func (m whyTUIModel) scrollListToCursor() whyTUIModel { + height := m.bodyHeight() + if m.cursor < m.listTop { + m.listTop = m.cursor + } + if m.cursor >= m.listTop+height { + m.listTop = m.cursor - height + 1 + } + if m.listTop < 0 { + m.listTop = 0 + } + return m +} + +// jumpAgentLine moves the cursor to the next/previous line with agent +// involvement ([AI] or [MX]) — browsing straight to the interesting lines. +func (m whyTUIModel) jumpAgentLine(dir int) whyTUIModel { + lines := m.result.Lines + for i := m.cursor + dir; i >= 0 && i < len(lines); i += dir { + if lines[i].Authorship == attributionAI || lines[i].Authorship == attributionMixed { + return m.moveCursor(i) + } + } + m.statusMsg = "no more agent-attributed lines" + return m +} + +func (m whyTUIModel) layout() whyTUIModel { + if m.width <= 0 || m.height <= 0 { + return m + } + bodyH := m.bodyHeight() + rightW := m.rightPaneWidth() + if !m.ready { + m.vp = viewport.New(viewport.WithWidth(rightW), viewport.WithHeight(bodyH)) + m.ready = true + } else { + m.vp.SetWidth(rightW) + m.vp.SetHeight(bodyH) + } + return m.refreshDetail() +} + +func (m whyTUIModel) bodyHeight() int { + h := m.height - whyHeaderHeight - whyFooterHeight + if h < 1 { + h = 1 + } + return h +} + +func (m whyTUIModel) listWidth() int { + w := int(float64(m.width) * whyListWidthRatio) + if w < whyListMinWidth { + w = whyListMinWidth + } + if limit := m.width - whyPaneGap - 20; w > limit { + w = limit + } + if w < 1 { + w = 1 + } + return w +} + +func (m whyTUIModel) rightPaneWidth() int { + w := m.width - m.listWidth() - whyPaneGap + if w < 1 { + w = 1 + } + return w +} + +func (m whyTUIModel) refreshDetail() whyTUIModel { + m.statusMsg = "" + if !m.ready { + return m + } + m.vp.SetContent(m.renderDetail(m.rightPaneWidth())) + m.vp.GotoTop() + return m +} + +// selectedLine returns the line under the cursor, or nil for an empty file. +func (m whyTUIModel) selectedLine() *attributionLine { + if m.cursor < 0 || m.cursor >= len(m.result.Lines) { + return nil + } + return &m.result.Lines[m.cursor] +} + +// sessionWebURL builds the entire.io session URL for the selected line, or "" +// when there is nothing to link. +func (m whyTUIModel) sessionWebURL(line *attributionLine) string { + if line == nil || m.repoFullName == "" || line.SessionID == "" { + return "" + } + owner, repo, ok := strings.Cut(m.repoFullName, "/") + if !ok || owner == "" || repo == "" { + return "" + } + return fmt.Sprintf("https://entire.io/gh/%s/%s/session/%s", + url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(line.SessionID)) +} + +func (m whyTUIModel) View() tea.View { + if !m.ready { + return tea.NewView("") + } + var b strings.Builder + summary := m.result.Summary + fmt.Fprintf(&b, "%s %s %s\n\n", + m.styles.render(m.styles.title, "why"), + m.styles.render(m.styles.file, m.result.File), + m.styles.render(m.styles.dim, fmt.Sprintf("%d lines · %d%% AI · %d%% human · %d%% mixed", + summary.TotalLines, summary.AIPercentage, summary.HumanPercentage, summary.MixedPercentage))) + + list := m.renderList(m.listWidth(), m.bodyHeight()) + detail := strings.Split(m.vp.View(), "\n") + sep := m.styles.render(m.styles.sepBar, "│") + for i := range m.bodyHeight() { + left, right := "", "" + if i < len(list) { + left = list[i] + } + if i < len(detail) { + right = detail[i] + } + fmt.Fprintf(&b, "%-*s %s %s\n", m.listWidth(), left, sep, right) + } + + legend := whyMarkerLegend + if m.statusMsg != "" { + legend = m.statusMsg + } + b.WriteString(m.styles.render(m.styles.dim, legend) + "\n") + b.WriteString(m.renderHelp()) + return tea.NewView(b.String()) +} + +// renderList renders the visible window of file lines, keeping the cursor in +// view. Lines are windowed manually (a file can be thousands of lines). +func (m whyTUIModel) renderList(width, height int) []string { + lines := m.result.Lines + if len(lines) == 0 { + return []string{m.styles.render(m.styles.dim, "(empty file)")} + } + + // Read-only clamp: the persisted window is maintained by + // scrollListToCursor; this only guards against a resize shrinking the + // window after the last cursor move. + top := m.listTop + if m.cursor < top { + top = m.cursor + } + if m.cursor >= top+height { + top = m.cursor - height + 1 + } + if top < 0 { + top = 0 + } + + numW := len(strconv.Itoa(lines[len(lines)-1].LineNumber)) + out := make([]string, 0, height) + for i := top; i < len(lines) && len(out) < height; i++ { + line := lines[i] + tag := attributionTag(line.Authorship) + marker := attributionLineMarker(line) + if marker == "" { + marker = " " + } + content := stringutil.TruncateRunes(strings.ReplaceAll(line.Content, "\t", " "), width-numW-9, "…") + row := fmt.Sprintf("%*d %s%s %s", numW, line.LineNumber, + m.styles.render(m.styles.tagStyle(tag), tag), marker, content) + if i == m.cursor { + row = m.styles.render(m.styles.selected, fmt.Sprintf("%*d %s%s %s", numW, line.LineNumber, tag, marker, content)) + } + out = append(out, stringutil.TruncateRunes(row, width+64, "")) // guard against style overshoot + } + return out +} + +// renderDetail builds the explanation pane for the selected line — the same +// facts as the plain-text `why :` output, formatted for a pane. +func (m whyTUIModel) renderDetail(width int) string { + line := m.selectedLine() + if line == nil { + return m.styles.render(m.styles.dim, "No lines.") + } + wrap := func(s string) string { + return lipgloss.NewStyle().Width(width).Render(s) + } + var b strings.Builder + sec := func(title string) { b.WriteString(m.styles.render(m.styles.section, title) + "\n") } + + sec(fmt.Sprintf("LINE %d %s", line.LineNumber, attributionTag(line.Authorship))) + b.WriteString(wrap(m.authorshipSentence(line)) + "\n\n") + + if line.ShortCommitSHA != "" { + commit := "Commit: " + line.ShortCommitSHA + if line.Author != "" { + commit += " by " + line.Author + } + if line.AuthorTime != nil { + commit += " " + line.AuthorTime.Format("2006-01-02 15:04") + } + b.WriteString(wrap(commit) + "\n") + } + if line.Agent != "" { + agentLine := "Agent: " + line.Agent + if line.Model != "" { + agentLine += " · " + line.Model + } + b.WriteString(wrap(agentLine) + "\n") + } + if line.SessionID != "" { + sessionText := "Session: " + shortSessionID(line.SessionID) + if u := m.sessionWebURL(line); u != "" { + sessionText = m.link(m.styles.dim, u, sessionText) // dim styled + clickable + } + b.WriteString(wrap(sessionText) + "\n") + } + if line.CheckpointID != "" { + b.WriteString(wrap("Checkpoint: "+line.CheckpointID) + "\n") + } + b.WriteString("\n") + + if line.Prompt != "" { + label := "PROMPT" + if line.PromptSessionLevel { + label = "SESSION PROMPT" + } + sec(label) + prompt := line.Prompt + if !m.expanded { + prompt = stringutil.TruncateRunes(stringutil.CollapseWhitespace(prompt), 240, "… (enter to expand)") + } + b.WriteString(wrap(prompt) + "\n") + if line.PromptSessionLevel { + b.WriteString(wrap(m.styles.render(m.styles.dim, "session-level prompt — may not appear in this checkpoint's transcript")) + "\n") + } + b.WriteString("\n") + } + if line.Intent != "" && line.Intent != line.Prompt { + sec("INTENT") + b.WriteString(wrap(line.Intent) + "\n\n") + } + + if line.MetadataMissing { + msg := "Checkpoint metadata was not found locally; showing trailer-level attribution only." + if line.MetadataMissingReason != "" { + msg = line.MetadataMissingReason + } + b.WriteString(wrap(m.styles.render(m.styles.warn, msg)) + "\n\n") + } + if line.SessionFallback { + b.WriteString(wrap(m.styles.render(m.styles.warn, + "~ best-effort: this file is not in the checkpoint session's recorded paths; the agent and prompt shown are a guess")) + "\n\n") + } + + if len(line.Candidates) > 1 { + sec(fmt.Sprintf("CANDIDATE CHECKPOINTS (%d)", len(line.Candidates))) + for _, c := range line.Candidates { + row := "- " + c.CheckpointID + if c.Agent != "" { + row += " · " + c.Agent + } + if m.expanded && c.Prompt != "" { + row += " · " + stringutil.TruncateRunes(stringutil.CollapseWhitespace(c.Prompt), 120, "…") + } + b.WriteString(wrap(row) + "\n") + } + b.WriteString("\n") + } + + if line.CheckpointID != "" && !line.MetadataMissing { + b.WriteString(wrap(m.styles.render(m.styles.dim, "Full context: entire checkpoint explain "+line.CheckpointID)) + "\n") + } + return b.String() +} + +// authorshipSentence spells out what the tag means for THIS line, with the +// wording the attribution actually supports: [AI] = the commit's checkpointed +// work was fully agent-authored; [MX] = agent work with human edits mixed in; +// [HU] = no agent checkpoint recorded for the commit. +func (m whyTUIModel) authorshipSentence(line *attributionLine) string { + switch line.Authorship { + case attributionAI: + return "Agent-authored: the checkpoint work behind this commit was fully agent-authored." + case attributionMixed: + return "Mixed: this commit combined agent work with human edits, so this line may be either." + case attributionUncommitted: + return "Uncommitted: this line has no commit yet." + case attributionHuman: + return "Human: no agent checkpoint is recorded for this commit." + default: + return string(line.Authorship) + } +} + +// link is a tiny alias so renderDetail reads naturally. +func (m whyTUIModel) link(style lipgloss.Style, url, text string) string { + return m.styles.link(style, url, text) +} + +func (m whyTUIModel) renderHelp() string { + parts := []struct{ k, d string }{ + {"↑/↓", "line"}, {"n/p", "next/prev agent line"}, {"enter", "expand"}, + {"pgup/pgdn", "page"}, {"g/G", "top/bottom"}, {"q", "quit"}, + } + var b strings.Builder + for i, p := range parts { + if i > 0 { + b.WriteString(m.styles.render(m.styles.helpDesc, " · ")) + } + b.WriteString(m.styles.render(m.styles.helpKey, p.k) + " " + m.styles.render(m.styles.helpDesc, p.d)) + } + return b.String() +} diff --git a/cmd/entire/cli/attribution_tui_test.go b/cmd/entire/cli/attribution_tui_test.go new file mode 100644 index 0000000000..afa12488bc --- /dev/null +++ b/cmd/entire/cli/attribution_tui_test.go @@ -0,0 +1,280 @@ +package cli + +import ( + "bytes" + "strings" + "testing" + "time" + + tea "charm.land/bubbletea/v2" +) + +func whyTUIFixture() *fileAttributionResult { + when := time.Date(2026, 1, 2, 15, 4, 0, 0, time.UTC) + lines := []attributionLine{ + { + LineNumber: 1, Authorship: attributionHuman, Tag: "[HU]", + CommitSHA: "1111111111111111111111111111111111111111", ShortCommitSHA: "1111111", + Author: "Ada", AuthorTime: &when, Content: "package main", + }, + { + LineNumber: 2, Authorship: attributionAI, Tag: "[AI]", + CommitSHA: "2222222222222222222222222222222222222222", ShortCommitSHA: "2222222", + Author: "Ada", AuthorTime: &when, + CheckpointID: "a1b2c3d4e5f6", SessionID: "session-agent-12345678", + Agent: "Claude Code", Model: "claude-test", + Prompt: "Fix the authentication bug in login flow please", + Intent: "Fix auth bug", + Content: "func login() {}", + }, + { + LineNumber: 3, Authorship: attributionMixed, Tag: "[MX]", + CommitSHA: "3333333333333333333333333333333333333333", ShortCommitSHA: "3333333", + CheckpointID: "b1b2c3d4e5f6", SessionID: "session-mixed-12345678", + Agent: "Claude Code", Prompt: "Refactor helpers", PromptSessionLevel: true, + Candidates: []attributionCandidate{ + {CheckpointID: "b1b2c3d4e5f6", Agent: "Claude Code", Prompt: "Refactor helpers"}, + {CheckpointID: "c1b2c3d4e5f6", Agent: "Codex", Prompt: "Tidy up"}, + }, + Content: "func helper() {}", + }, + { + LineNumber: 4, Authorship: attributionAI, Tag: "[AI]", + CheckpointID: "d1b2c3d4e5f6", MetadataMissing: true, + MetadataMissingReason: "checkpoint metadata was not found locally. Run: git fetch origin entire/checkpoints/v1:entire/checkpoints/v1.", + Content: "func missing() {}", + }, + } + return &fileAttributionResult{ + File: "auth.py", + Lines: lines, + Summary: attributionSummary{ + TotalLines: 4, AILines: 2, HumanLines: 1, MixedLines: 1, + AIPercentage: 50, HumanPercentage: 25, MixedPercentage: 25, + }, + } +} + +func updateWhyTUI(t *testing.T, m whyTUIModel, msg tea.Msg) whyTUIModel { + t.Helper() + next, _ := m.Update(msg) + tm, ok := next.(whyTUIModel) + if !ok { + t.Fatalf("Update returned %T, want whyTUIModel", next) + } + return tm +} + +// Color off keeps assertions on raw text; a tall window keeps the detail pane +// fully visible so assertions aren't tripped by viewport scrolling. +func newSizedWhyTUI(t *testing.T, result *fileAttributionResult, startLine int) whyTUIModel { + t.Helper() + m := newWhyTUIModel(result, "acme/app", false, startLine) + return updateWhyTUI(t, m, tea.WindowSizeMsg{Width: 140, Height: 44}) +} + +func whyTUIViewText(t *testing.T, m whyTUIModel) string { + t.Helper() + return m.View().Content +} + +func keyPress(k string) tea.KeyPressMsg { + switch k { + case "up": + return tea.KeyPressMsg{Code: tea.KeyUp} + case "down": + return tea.KeyPressMsg{Code: tea.KeyDown} + case "enter": + return tea.KeyPressMsg{Code: tea.KeyEnter} + default: + r := []rune(k)[0] + return tea.KeyPressMsg{Code: r, Text: k} + } +} + +func TestWhyTUIRendersFileAndSelectedLineDetail(t *testing.T) { + t.Parallel() + m := newSizedWhyTUI(t, whyTUIFixture(), 0) + text := whyTUIViewText(t, m) + + for _, want := range []string{ + "why", "auth.py", "4 lines", "50% AI", + "package main", "func login() {}", // list content + "LINE 1", "[HU]", "Human: no agent checkpoint", // detail for initial cursor (line 1) + whyMarkerLegend, // footer legend + "quit", // help line + } { + if !strings.Contains(text, want) { + t.Fatalf("TUI view missing %q:\n%s", want, text) + } + } +} + +func TestWhyTUINavigationShowsPromptDetail(t *testing.T) { + t.Parallel() + m := newSizedWhyTUI(t, whyTUIFixture(), 0) + m = updateWhyTUI(t, m, keyPress("down")) // to line 2 ([AI]) + text := whyTUIViewText(t, m) + + for _, want := range []string{ + "LINE 2", "[AI]", "fully agent-authored", + "Agent: Claude Code · claude-test", + "Session: session-", "Checkpoint: a1b2c3d4e5f6", + "PROMPT", "Fix the authentication bug", + "INTENT", "Fix auth bug", + "Full context: entire checkpoint explain a1b2c3d4e5f6", + } { + if !strings.Contains(text, want) { + t.Fatalf("after down, view missing %q:\n%s", want, text) + } + } +} + +func TestWhyTUIStartLinePositionsCursor(t *testing.T) { + t.Parallel() + m := newSizedWhyTUI(t, whyTUIFixture(), 3) + text := whyTUIViewText(t, m) + + for _, want := range []string{ + "LINE 3", "[MX]", "combined agent work with human edits", + "SESSION PROMPT", "session-level prompt", + "CANDIDATE CHECKPOINTS (2)", "c1b2c3d4e5f6", "Codex", + } { + if !strings.Contains(text, want) { + t.Fatalf("start-line view missing %q:\n%s", want, text) + } + } +} + +func TestWhyTUIJumpToNextAgentLine(t *testing.T) { + t.Parallel() + m := newSizedWhyTUI(t, whyTUIFixture(), 0) // cursor at line 1 [HU] + m = updateWhyTUI(t, m, keyPress("n")) // -> line 2 [AI] + if got := m.selectedLine().LineNumber; got != 2 { + t.Fatalf("n jumped to line %d, want 2", got) + } + m = updateWhyTUI(t, m, keyPress("n")) // -> line 3 [MX] + m = updateWhyTUI(t, m, keyPress("n")) // -> line 4 [AI] + if got := m.selectedLine().LineNumber; got != 4 { + t.Fatalf("n n jumped to line %d, want 4", got) + } + // No further agent lines: cursor stays, status message appears. + m = updateWhyTUI(t, m, keyPress("n")) + if got := m.selectedLine().LineNumber; got != 4 { + t.Fatalf("n at end moved to line %d, want 4", got) + } + if !strings.Contains(whyTUIViewText(t, m), "no more agent-attributed lines") { + t.Fatal("expected status message about no more agent lines") + } + // p goes back. + m = updateWhyTUI(t, m, keyPress("p")) + if got := m.selectedLine().LineNumber; got != 3 { + t.Fatalf("p jumped to line %d, want 3", got) + } +} + +func TestWhyTUIExpandTogglesFullPrompt(t *testing.T) { + t.Parallel() + long := strings.Repeat("very long prompt text ", 30) + result := whyTUIFixture() + result.Lines[1].Prompt = long + m := newSizedWhyTUI(t, result, 2) + + if !strings.Contains(whyTUIViewText(t, m), "enter to expand") { + t.Fatal("collapsed view should hint at expansion") + } + m = updateWhyTUI(t, m, keyPress("enter")) + if strings.Contains(whyTUIViewText(t, m), "enter to expand") { + t.Fatal("expanded view should not truncate the prompt") + } +} + +func TestWhyTUIMissingMetadataShowsReason(t *testing.T) { + t.Parallel() + m := newSizedWhyTUI(t, whyTUIFixture(), 4) + text := whyTUIViewText(t, m) + if !strings.Contains(text, "checkpoint metadata was not found locally") { + t.Fatalf("missing-metadata reason absent:\n%s", text) + } + if strings.Contains(text, "Full context: entire checkpoint explain d1b2c3d4e5f6") { + t.Fatal("explain hint must be suppressed when metadata is missing") + } +} + +// g/G map to the shared keymap's Home/End bindings. +func TestWhyTUIHomeEndKeys(t *testing.T) { + t.Parallel() + m := newSizedWhyTUI(t, whyTUIFixture(), 3) // start mid-file + m = updateWhyTUI(t, m, keyPress("g")) + if got := m.selectedLine().LineNumber; got != 1 { + t.Fatalf("g moved to line %d, want 1", got) + } + m = updateWhyTUI(t, m, keyPress("G")) + if got := m.selectedLine().LineNumber; got != 4 { + t.Fatalf("G moved to line %d, want 4", got) + } +} + +func TestWhyTUIQuitKeys(t *testing.T) { + t.Parallel() + m := newSizedWhyTUI(t, whyTUIFixture(), 0) + for _, k := range []string{"q"} { + _, cmd := m.Update(keyPress(k)) + if cmd == nil { + t.Fatalf("key %q should quit", k) + } + } +} + +func TestWhyTUIEmptyFileDoesNotPanic(t *testing.T) { + t.Parallel() + empty := &fileAttributionResult{File: "empty.py", Lines: nil} + m := newWhyTUIModel(empty, "", false, 0) + m = updateWhyTUI(t, m, tea.WindowSizeMsg{Width: 80, Height: 20}) + m = updateWhyTUI(t, m, keyPress("down")) + m = updateWhyTUI(t, m, keyPress("n")) + text := whyTUIViewText(t, m) + if !strings.Contains(text, "empty.py") { + t.Fatalf("empty-file view missing filename:\n%s", text) + } +} + +func TestWhyTUITinyWindowDoesNotPanic(t *testing.T) { + t.Parallel() + m := newWhyTUIModel(whyTUIFixture(), "", false, 0) + m = updateWhyTUI(t, m, tea.WindowSizeMsg{Width: 8, Height: 3}) + _ = whyTUIViewText(t, m) + m = updateWhyTUI(t, m, keyPress("down")) + _ = whyTUIViewText(t, m) +} + +// The agent-safe fallback contract: --tui against a non-TTY writer must fall +// through to the deterministic plain-text output (never start the TUI, never +// block). A bytes.Buffer is the non-TTY case IsTerminalWriter reports false for. +func TestWhyTUIFlagFallsBackToPlainTextWhenNotTTY(t *testing.T) { + newAttributionRepo(t) + + var buf bytes.Buffer + err := runAttributionWhy(t.Context(), &buf, "auth.py", attributionWhyOptions{TUI: true}) + if err != nil { + t.Fatalf("runAttributionWhy --tui non-TTY: %v", err) + } + out := buf.String() + if !strings.Contains(out, "auth.py") || !strings.Contains(out, "lines") { + t.Fatalf("expected the plain file summary as fallback, got:\n%s", out) + } +} + +// --json must win over --tui so scripted callers always get JSON. +func TestWhyTUIFlagJSONWins(t *testing.T) { + newAttributionRepo(t) + + var buf bytes.Buffer + err := runAttributionWhy(t.Context(), &buf, "auth.py", attributionWhyOptions{TUI: true, JSON: true}) + if err != nil { + t.Fatalf("runAttributionWhy --tui --json: %v", err) + } + if !strings.HasPrefix(strings.TrimSpace(buf.String()), "{") { + t.Fatalf("expected JSON output, got:\n%s", buf.String()) + } +} From 043702ba820d8dbf2ba81e1bad54f75c5d7120a7 Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Thu, 2 Jul 2026 17:58:46 -0400 Subject: [PATCH 18/19] fix(attribution): legend states tags are per-commit inference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The [AI]/[MX]/[HU] tags are a per-commit inference, not a per-line truth: [AI] means the commit's checkpointed work was fully agent-authored, [MX] means the commit mixed agent and human edits (any given line may be either), [HU] means no agent checkpoint. The legend now says so explicitly — 'per commit: [AI] all agent · [MX] mixed — line may be either · [HU] no agent' — instead of reading like per-line labels. [??] moved to the conditional marker legend line (shown only when uncommitted lines exist), keeping the main legend inside the blame table's 80-column budget. --- cmd/entire/cli/attribution.go | 10 ++++++++-- cmd/entire/cli/attribution_tui.go | 14 ++++++++------ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/cmd/entire/cli/attribution.go b/cmd/entire/cli/attribution.go index d191020f02..04a6f8dcc9 100644 --- a/cmd/entire/cli/attribution.go +++ b/cmd/entire/cli/attribution.go @@ -1288,7 +1288,7 @@ func renderAttributionMarkerLegend(w io.Writer, sty statusStyles, lines []attrib // with human edits mixed in at commit, [HU] means no agent checkpoint. fmt.Fprintf(w, " %s\n", sty.render(sty.dim, whyMarkerLegend)) - approximate, ambiguous := false, false + approximate, ambiguous, uncommitted := false, false, false for _, line := range lines { switch attributionLineMarker(line) { case "~": @@ -1296,11 +1296,17 @@ func renderAttributionMarkerLegend(w io.Writer, sty statusStyles, lines []attrib case "?": ambiguous = true } + if line.Authorship == attributionUncommitted { + uncommitted = true + } } - if !approximate && !ambiguous { + if !approximate && !ambiguous && !uncommitted { return } var parts []string + if uncommitted { + parts = append(parts, "[??] uncommitted (no commit yet)") + } if approximate { parts = append(parts, "~ best-effort attribution (file not in the checkpoint's recorded paths)") } diff --git a/cmd/entire/cli/attribution_tui.go b/cmd/entire/cli/attribution_tui.go index 440fb6838c..290ede7e8f 100644 --- a/cmd/entire/cli/attribution_tui.go +++ b/cmd/entire/cli/attribution_tui.go @@ -27,12 +27,14 @@ const ( ) // whyMarkerLegend is the one-line explanation of the attribution tags shown in -// the footer and in the plain-text views. Wording is deliberate: [AI] means the -// commit's checkpointed work was fully agent-authored; [MX] means agent work -// with human edits mixed in at commit; [HU] means no agent checkpoint recorded. -// Kept within the blame table's 80-column budget (with its 2-space indent); the -// full sentences live in the why detail views. -const whyMarkerLegend = "[AI] all agent · [MX] agent+human mixed · [HU] no agent · [??] uncommitted" +// the footer and in the plain-text views. Wording is deliberate — the tags are +// a PER-COMMIT inference, not a per-line truth: [AI] means the commit's +// checkpointed work was fully agent-authored; [MX] means the commit mixed +// agent work with human edits (so any given line may be either); [HU] means no +// agent checkpoint is recorded for the commit. Kept within the blame table's +// 80-column budget (with its 2-space indent); full sentences live in the why +// detail views, and [??]/~/? markers are explained by their own legend line. +const whyMarkerLegend = "per commit: [AI] all agent · [MX] mixed — line may be either · [HU] no agent" // whyTUIStyles holds the interactive viewer's palette. Empty styles render as // plain text when color is off, which also keeps tests assertable. From e31f61985a8439d841baac47ce36739e5b07a843 Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Thu, 2 Jul 2026 18:22:59 -0400 Subject: [PATCH 19/19] fix(attribution): spell out the [AI]/[MX] inference rule in the legend MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rule people do not guess: a single human edit anywhere in a commit turns every line of it [MX] — [AI] appears only for fully agent-authored commits. The blame and why legends now carry an explicit second note line saying so: 'note: [AI] requires a fully agent commit; one human edit turns all lines [MX]'. Within the blame table's 80-column budget. --- cmd/entire/cli/attribution.go | 1 + cmd/entire/cli/attribution_tui.go | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/cmd/entire/cli/attribution.go b/cmd/entire/cli/attribution.go index 04a6f8dcc9..3d02c68bc9 100644 --- a/cmd/entire/cli/attribution.go +++ b/cmd/entire/cli/attribution.go @@ -1287,6 +1287,7 @@ func renderAttributionMarkerLegend(w io.Writer, sty statusStyles, lines []attrib // [AI] means fully agent-authored checkpoint work, [MX] means agent work // with human edits mixed in at commit, [HU] means no agent checkpoint. fmt.Fprintf(w, " %s\n", sty.render(sty.dim, whyMarkerLegend)) + fmt.Fprintf(w, " %s\n", sty.render(sty.dim, whyMarkerLegendNote)) approximate, ambiguous, uncommitted := false, false, false for _, line := range lines { diff --git a/cmd/entire/cli/attribution_tui.go b/cmd/entire/cli/attribution_tui.go index 290ede7e8f..c9d0f0d53f 100644 --- a/cmd/entire/cli/attribution_tui.go +++ b/cmd/entire/cli/attribution_tui.go @@ -36,6 +36,12 @@ const ( // detail views, and [??]/~/? markers are explained by their own legend line. const whyMarkerLegend = "per commit: [AI] all agent · [MX] mixed — line may be either · [HU] no agent" +// whyMarkerLegendNote spells out the inference rule people don't guess (a +// single human edit anywhere in a commit turns EVERY line of it [MX] — [AI] +// appears only for fully agent-authored commits). Rendered as a second dim +// line under the legend in the plain-text views. +const whyMarkerLegendNote = "note: [AI] requires a fully agent commit; one human edit turns all lines [MX]" + // whyTUIStyles holds the interactive viewer's palette. Empty styles render as // plain text when color is off, which also keeps tests assertable. type whyTUIStyles struct {