diff --git a/cmd/entire/cli/attach.go b/cmd/entire/cli/attach.go index 3ef88a9eb..e75a01f1c 100644 --- a/cmd/entire/cli/attach.go +++ b/cmd/entire/cli/attach.go @@ -170,11 +170,21 @@ func runAttach(ctx context.Context, w io.Writer, sessionID string, agentName typ writeOpts.CompactTranscript = compacted } - if err := store.WriteCommitted(ctx, writeOpts); err != nil { - return fmt.Errorf("failed to write checkpoint: %w", err) + v2Only := settings.IsCheckpointsV2OnlyEnabled(logCtx) + if !v2Only { + if err := store.WriteCommitted(ctx, writeOpts); err != nil { + return fmt.Errorf("failed to write checkpoint: %w", err) + } } + // IsCheckpointsV2Enabled is true whenever v2Only is true, so this covers both + // the v2-only and dual-write paths. Only v2-only propagates the error. if settings.IsCheckpointsV2Enabled(logCtx) { - writeAttachCheckpointV2(logCtx, repo, writeOpts) + if err := writeAttachCheckpointV2(logCtx, repo, writeOpts); err != nil { + if v2Only { + return fmt.Errorf("failed to write checkpoint to v2: %w", err) + } + logging.Warn(logCtx, "attach v2 dual-write failed", "error", err) + } } // Create or update session state. @@ -198,17 +208,13 @@ func runAttach(ctx context.Context, w io.Writer, sessionID string, agentName typ return nil } -// writeAttachCheckpointV2 mirrors attach-created checkpoints into the v2 refs. -// The caller is responsible for checking whether checkpoints_v2 is enabled. -// v2 failures are logged and do not fail attach. -func writeAttachCheckpointV2(ctx context.Context, repo *git.Repository, opts cpkg.WriteCommittedOptions) { +// writeAttachCheckpointV2 writes attach-created checkpoints into the v2 refs. +func writeAttachCheckpointV2(ctx context.Context, repo *git.Repository, opts cpkg.WriteCommittedOptions) error { v2Store := cpkg.NewV2GitStore(repo, strategy.ResolveCheckpointURL(ctx, "origin")) if err := v2Store.WriteCommitted(ctx, opts); err != nil { - logging.Warn(ctx, "attach v2 dual-write failed", - "checkpoint_id", opts.CheckpointID.String(), - "error", err, - ) + return fmt.Errorf("v2 write committed: %w", err) } + return nil } // getHeadCommit returns the HEAD commit object. diff --git a/cmd/entire/cli/checkpoint/v2_read.go b/cmd/entire/cli/checkpoint/v2_read.go index ec011ea5c..95dd305ec 100644 --- a/cmd/entire/cli/checkpoint/v2_read.go +++ b/cmd/entire/cli/checkpoint/v2_read.go @@ -495,6 +495,33 @@ func readTranscriptFromObjectTree(tree *object.Tree, agentType types.AgentType) return nil, nil } +// ReadSessionContentByID finds the session with the given sessionID in a checkpoint +// and returns its content. Mirrors GitStore.ReadSessionContentByID for v2 refs. +// Returns ErrCheckpointNotFound if the checkpoint doesn't exist; returns a +// non-wrapped error (containing the session ID and checkpoint ID for context) +// if no session in the checkpoint matches sessionID. +func (s *V2GitStore) ReadSessionContentByID(ctx context.Context, checkpointID id.CheckpointID, sessionID string) (*SessionContent, error) { + summary, err := s.ReadCommitted(ctx, checkpointID) + if err != nil { + return nil, err + } + if summary == nil { + return nil, ErrCheckpointNotFound + } + + for i := range summary.Sessions { + content, readErr := s.ReadSessionContent(ctx, checkpointID, i) + if readErr != nil { + continue + } + if content != nil && content.Metadata.SessionID == sessionID { + return content, nil + } + } + + return nil, fmt.Errorf("session %q not found in checkpoint %s", sessionID, checkpointID) +} + // GetSessionLog reads the latest session's raw transcript and session ID from v2 refs. // Convenience wrapper matching the GitStore.GetSessionLog signature. func (s *V2GitStore) GetSessionLog(ctx context.Context, cpID id.CheckpointID) ([]byte, string, error) { diff --git a/cmd/entire/cli/hooks_git_cmd_test.go b/cmd/entire/cli/hooks_git_cmd_test.go index 790beb30f..a15f37454 100644 --- a/cmd/entire/cli/hooks_git_cmd_test.go +++ b/cmd/entire/cli/hooks_git_cmd_test.go @@ -256,6 +256,7 @@ func TestHooksGitCmd_ExposesPostRewriteSubcommand(t *testing.T) { } if found == nil { t.Fatal("expected post-rewrite subcommand, got nil") + return } if found.Use != "post-rewrite " { t.Fatalf("post-rewrite Use = %q, want %q", found.Use, "post-rewrite ") diff --git a/cmd/entire/cli/integration_test/v2_dual_write_test.go b/cmd/entire/cli/integration_test/v2_dual_write_test.go index 5d40d1c6d..d491fcc94 100644 --- a/cmd/entire/cli/integration_test/v2_dual_write_test.go +++ b/cmd/entire/cli/integration_test/v2_dual_write_test.go @@ -244,3 +244,52 @@ func TestV2DualWrite_StopTimeFinalization(t *testing.T) { require.True(t, found, "transcript.jsonl should exist on v2 /main after finalization") assert.Contains(t, compactTranscript, `"v":1`) } + +// TestV2Only_SkipsV1Write verifies the v2-only specific deltas: v1 metadata is +// not written and v2 refs still exist. The full v2 payload shape is already +// covered by TestV2DualWrite_FullWorkflow. +func TestV2Only_SkipsV1Write(t *testing.T) { + t.Parallel() + env := NewTestEnv(t) + defer env.Cleanup() + + env.InitRepo() + env.WriteFile("README.md", "# Test") + env.WriteFile(".gitignore", ".entire/\n") + env.GitAdd("README.md") + env.GitAdd(".gitignore") + env.GitCommit("Initial commit") + env.GitCheckoutNewBranch("feature/v2-only-test") + + env.InitEntireWithOptions(map[string]any{ + "checkpoints_v2_only": true, + }) + + session := env.NewSession() + require.NoError(t, env.SimulateUserPromptSubmitWithPrompt(session.ID, "Add greeting function")) + + env.WriteFile("greet.go", "package main\n\nfunc Greet() string { return \"hello\" }") + session.CreateTranscript( + "Add greeting function", + []FileChange{{Path: "greet.go", Content: "package main\n\nfunc Greet() string { return \"hello\" }"}}, + ) + require.NoError(t, env.SimulateStop(session.ID, session.TranscriptPath)) + + env.GitCommitWithShadowHooks("Add greeting function", "greet.go") + + cpIDStr := env.GetLatestCheckpointIDFromHistory() + require.NotEmpty(t, cpIDStr, "checkpoint ID should be in commit trailer") + + cpID, err := id.NewCheckpointID(cpIDStr) + require.NoError(t, err) + cpPath := cpID.Path() + + // v1: should NOT be written. + _, found := env.ReadFileFromBranch(paths.MetadataBranchName, cpPath+"/"+paths.MetadataFileName) + assert.False(t, found, + "v1 committed checkpoint metadata should NOT exist when checkpoints_v2_only is enabled") + + // v2: smoke check that the checkpoint still landed. + assert.True(t, env.RefExists(paths.V2MainRefName), "v2 /main ref should exist") + assert.True(t, env.RefExists(paths.V2FullCurrentRefName), "v2 /full/current ref should exist") +} diff --git a/cmd/entire/cli/integration_test/v2_push_test.go b/cmd/entire/cli/integration_test/v2_push_test.go index f47d8d730..d42d22358 100644 --- a/cmd/entire/cli/integration_test/v2_push_test.go +++ b/cmd/entire/cli/integration_test/v2_push_test.go @@ -123,3 +123,47 @@ func TestV2Push_Disabled_NoV2Refs(t *testing.T) { assert.True(t, bareRefExists(t, bareDir, "refs/heads/"+paths.MetadataBranchName), "v1 metadata branch should still exist on remote") } + +// TestV2Push_V2OnlySkipsV1Branch verifies that the v1 metadata branch is not +// pushed when checkpoints_v2_only is enabled; v2 ref pushing itself is covered +// by TestV2Push_FullCycle. +func TestV2Push_V2OnlySkipsV1Branch(t *testing.T) { + t.Parallel() + env := NewTestEnv(t) + defer env.Cleanup() + + env.InitRepo() + env.WriteFile("README.md", "# Test") + env.WriteFile(".gitignore", ".entire/\n") + env.GitAdd("README.md") + env.GitAdd(".gitignore") + env.GitCommit("Initial commit") + env.GitCheckoutNewBranch("feature/v2-only-push-test") + + env.InitEntireWithOptions(map[string]any{ + "checkpoints_v2_only": true, + }) + + bareDir := env.SetupBareRemote() + + session := env.NewSession() + require.NoError(t, env.SimulateUserPromptSubmitWithPrompt(session.ID, "Add feature")) + + env.WriteFile("feature.go", "package main\n\nfunc Feature() {}") + session.CreateTranscript( + "Add feature", + []FileChange{{Path: "feature.go", Content: "package main\n\nfunc Feature() {}"}}, + ) + require.NoError(t, env.SimulateStop(session.ID, session.TranscriptPath)) + + env.GitAdd("feature.go") + env.GitCommitWithShadowHooks("Add feature") + + env.RunPrePush("origin") + + assert.False(t, bareRefExists(t, bareDir, "refs/heads/"+paths.MetadataBranchName), + "v1 metadata branch should NOT exist on remote when checkpoints_v2_only is enabled") + // Smoke: v2 refs still land; full payload asserted in TestV2Push_FullCycle. + assert.True(t, bareRefExists(t, bareDir, paths.V2MainRefName), + "v2 /main ref should exist on remote after push") +} diff --git a/cmd/entire/cli/settings/settings.go b/cmd/entire/cli/settings/settings.go index d3bf99629..45d43922d 100644 --- a/cmd/entire/cli/settings/settings.go +++ b/cmd/entire/cli/settings/settings.go @@ -567,6 +567,16 @@ func IsCheckpointsV2Enabled(ctx context.Context) bool { return settings.IsCheckpointsV2Enabled() } +// IsCheckpointsV2OnlyEnabled checks if checkpoints should be written and pushed +// only via v2 refs. +func IsCheckpointsV2OnlyEnabled(ctx context.Context) bool { + s, err := Load(ctx) + if err != nil { + return false + } + return s.IsCheckpointsV2OnlyEnabled() +} + // IsPushV2RefsEnabled checks if pushing v2 refs is enabled in settings. // Returns false by default if settings cannot be loaded or flags are missing. func IsPushV2RefsEnabled(ctx context.Context) bool { @@ -658,9 +668,12 @@ func (s *EntireSettings) GetCheckpointRemote() *CheckpointRemoteConfig { return &CheckpointRemoteConfig{Provider: provider, Repo: repo} } -// IsCheckpointsV2Enabled checks if checkpoints v2 (dual-write to refs/entire/) is enabled. -// Returns false by default if the key is missing or not a bool. +// IsCheckpointsV2Enabled checks if checkpoints v2 is enabled. +// Returns true when either checkpoints_v2 or checkpoints_v2_only is enabled. func (s *EntireSettings) IsCheckpointsV2Enabled() bool { + if s.IsCheckpointsV2OnlyEnabled() { + return true + } if s.StrategyOptions == nil { return false } @@ -668,9 +681,22 @@ func (s *EntireSettings) IsCheckpointsV2Enabled() bool { return ok && val } +// IsCheckpointsV2OnlyEnabled checks if checkpoints should be written and pushed +// only via v2 refs, with no v1 dual-write. +func (s *EntireSettings) IsCheckpointsV2OnlyEnabled() bool { + if s.StrategyOptions == nil { + return false + } + val, ok := s.StrategyOptions["checkpoints_v2_only"].(bool) + return ok && val +} + // IsPushV2RefsEnabled checks if pushing v2 refs is enabled. -// Requires both checkpoints_v2 and push_v2_refs to be true. +// checkpoints_v2_only forces v2 ref pushes on, regardless of push_v2_refs. func (s *EntireSettings) IsPushV2RefsEnabled() bool { + if s.IsCheckpointsV2OnlyEnabled() { + return true + } if !s.IsCheckpointsV2Enabled() { return false } diff --git a/cmd/entire/cli/settings/settings_test.go b/cmd/entire/cli/settings/settings_test.go index 8b24ee7f9..041a9f509 100644 --- a/cmd/entire/cli/settings/settings_test.go +++ b/cmd/entire/cli/settings/settings_test.go @@ -688,6 +688,17 @@ func TestIsCheckpointsV2Enabled_True(t *testing.T) { } } +func TestIsCheckpointsV2Enabled_V2Only(t *testing.T) { + t.Parallel() + s := &EntireSettings{ + Enabled: true, + StrategyOptions: map[string]any{"checkpoints_v2_only": true}, + } + if !s.IsCheckpointsV2Enabled() { + t.Error("expected IsCheckpointsV2Enabled to be true when checkpoints_v2_only is enabled") + } +} + func TestIsCheckpointsV2Enabled_ExplicitlyFalse(t *testing.T) { t.Parallel() s := &EntireSettings{ @@ -773,6 +784,36 @@ func TestIsCheckpointsV2Enabled_LocalOverride(t *testing.T) { } } +func TestIsCheckpointsV2OnlyEnabled_DefaultsFalse(t *testing.T) { + t.Parallel() + s := &EntireSettings{Enabled: true} + if s.IsCheckpointsV2OnlyEnabled() { + t.Error("expected IsCheckpointsV2OnlyEnabled to default to false") + } +} + +func TestIsCheckpointsV2OnlyEnabled_True(t *testing.T) { + t.Parallel() + s := &EntireSettings{ + Enabled: true, + StrategyOptions: map[string]any{"checkpoints_v2_only": true}, + } + if !s.IsCheckpointsV2OnlyEnabled() { + t.Error("expected IsCheckpointsV2OnlyEnabled to be true") + } +} + +func TestIsCheckpointsV2OnlyEnabled_WrongType(t *testing.T) { + t.Parallel() + s := &EntireSettings{ + Enabled: true, + StrategyOptions: map[string]any{"checkpoints_v2_only": "yes"}, + } + if s.IsCheckpointsV2OnlyEnabled() { + t.Error("expected IsCheckpointsV2OnlyEnabled to be false for non-bool value") + } +} + func TestIsPushV2RefsEnabled_DefaultsFalse(t *testing.T) { t.Parallel() s := &EntireSettings{Enabled: true} @@ -789,6 +830,7 @@ func TestIsPushV2RefsEnabled_RequiresBothFlags(t *testing.T) { opts map[string]any expected bool }{ + {"v2 only supersedes both", map[string]any{"checkpoints_v2": false, "push_v2_refs": false, "checkpoints_v2_only": true}, true}, {"both true", map[string]any{"checkpoints_v2": true, "push_v2_refs": true}, true}, {"only checkpoints_v2", map[string]any{"checkpoints_v2": true}, false}, {"only push_v2_refs", map[string]any{"push_v2_refs": true}, false}, diff --git a/cmd/entire/cli/strategy/checkpoint_remote.go b/cmd/entire/cli/strategy/checkpoint_remote.go index 6f9d38979..e8411fd2e 100644 --- a/cmd/entire/cli/strategy/checkpoint_remote.go +++ b/cmd/entire/cli/strategy/checkpoint_remote.go @@ -119,17 +119,21 @@ func resolvePushSettings(ctx context.Context, pushRemoteName string) pushSetting ps.checkpointURL = checkpointURL - // If the checkpoint branch doesn't exist locally, try to fetch it from the URL. - // This is a one-time operation — once the branch exists locally, subsequent pushes - // skip the fetch entirely. Only fetch the metadata branch; trails are always pushed - // to the user's push remote, not the checkpoint remote. - if err := fetchMetadataBranchIfMissing(ctx, checkpointURL); err != nil { - logging.Warn(ctx, "checkpoint-remote: failed to fetch metadata branch", - slog.String("error", err.Error()), - ) + // Skip the v1 metadata-branch fetch entirely in v2-only mode — there is no + // v1 branch being written or pushed, so there is nothing to sync. + if !s.IsCheckpointsV2OnlyEnabled() { + // If the v1 checkpoint branch doesn't exist locally, try to fetch it from the URL. + // This is a one-time operation — once the branch exists locally, subsequent pushes + // skip the fetch entirely. Only fetch the metadata branch; trails are always pushed + // to the user's push remote, not the checkpoint remote. + if err := fetchMetadataBranchIfMissing(ctx, checkpointURL); err != nil { + logging.Warn(ctx, "checkpoint-remote: failed to fetch metadata branch", + slog.String("error", err.Error()), + ) + } } - // Also fetch v2 /main ref if push_v2_refs is enabled + // Also fetch v2 /main ref if v2 refs are enabled if s.IsPushV2RefsEnabled() { if err := fetchV2MainRefIfMissing(ctx, checkpointURL); err != nil { logging.Warn(ctx, "checkpoint-remote: failed to fetch v2 /main ref", diff --git a/cmd/entire/cli/strategy/manual_commit_condensation.go b/cmd/entire/cli/strategy/manual_commit_condensation.go index 1bc32abf6..c05149229 100644 --- a/cmd/entire/cli/strategy/manual_commit_condensation.go +++ b/cmd/entire/cli/strategy/manual_commit_condensation.go @@ -253,20 +253,32 @@ func (s *ManualCommitStrategy) CondenseSession(ctx context.Context, repo *git.Re compactTranscriptDuration := buildCompactTranscript(ctx, ag, redactedTranscript, state, &writeOpts) - // Write checkpoint metadata to v1 branch + v2Only := settings.IsCheckpointsV2OnlyEnabled(ctx) + + // Write checkpoint metadata to the primary store. writeV1Start := time.Now() writeCtx, writeCommittedSpan := perf.Start(ctx, "write_committed_v1") - if err := store.WriteCommitted(writeCtx, writeOpts); err != nil { - writeCommittedSpan.RecordError(err) - writeCommittedSpan.End() - return nil, fmt.Errorf("failed to write checkpoint metadata: %w", err) + if !v2Only { + if err := store.WriteCommitted(writeCtx, writeOpts); err != nil { + writeCommittedSpan.RecordError(err) + writeCommittedSpan.End() + return nil, fmt.Errorf("failed to write checkpoint metadata: %w", err) + } } writeCommittedSpan.End() writeV1Duration := time.Since(writeV1Start) writeV2Start := time.Now() writeV2Ctx, writeCommittedV2Span := perf.Start(ctx, "write_committed_v2") - writeCommittedV2IfEnabled(writeV2Ctx, repo, writeOpts) + if v2Only { + if err := writeCommittedV2(writeV2Ctx, repo, writeOpts); err != nil { + writeCommittedV2Span.RecordError(err) + writeCommittedV2Span.End() + return nil, fmt.Errorf("failed to write checkpoint metadata to v2: %w", err) + } + } else { + writeCommittedV2IfEnabled(writeV2Ctx, repo, writeOpts) + } writeTaskMetadataV2IfEnabled(writeV2Ctx, repo, checkpointID, state.SessionID, ref) writeCommittedV2Span.End() writeV2Duration := time.Since(writeV2Start) @@ -1397,16 +1409,24 @@ func computeCompactTranscriptStart(ctx context.Context, ag agent.Agent, state *S return offset } +// writeCommittedV2 writes checkpoint data to v2 refs unconditionally. +// Callers decide whether to propagate or swallow the error (v2-only vs dual-write). +func writeCommittedV2(ctx context.Context, repo *git.Repository, opts cpkg.WriteCommittedOptions) error { + v2Store := cpkg.NewV2GitStore(repo, ResolveCheckpointURL(ctx, "origin")) + if err := v2Store.WriteCommitted(ctx, opts); err != nil { + return fmt.Errorf("v2 write committed: %w", err) + } + return nil +} + // writeCommittedV2IfEnabled writes checkpoint data to v2 refs when checkpoints_v2 -// is enabled in settings. Failures are logged as warnings — v2 writes are -// best-effort during the dual-write period and must not block the v1 path. +// is enabled. Failures are logged as warnings — in dual-write mode v2 writes are +// best-effort and must not block the v1 path. func writeCommittedV2IfEnabled(ctx context.Context, repo *git.Repository, opts cpkg.WriteCommittedOptions) { if !settings.IsCheckpointsV2Enabled(ctx) { return } - - v2Store := cpkg.NewV2GitStore(repo, ResolveCheckpointURL(ctx, "origin")) - if err := v2Store.WriteCommitted(ctx, opts); err != nil { + if err := writeCommittedV2(ctx, repo, opts); err != nil { logging.Warn(ctx, "v2 dual-write failed", slog.String("checkpoint_id", opts.CheckpointID.String()), slog.String("error", err.Error()), diff --git a/cmd/entire/cli/strategy/manual_commit_hooks.go b/cmd/entire/cli/strategy/manual_commit_hooks.go index fcbc9c94f..5ab099e37 100644 --- a/cmd/entire/cli/strategy/manual_commit_hooks.go +++ b/cmd/entire/cli/strategy/manual_commit_hooks.go @@ -2661,6 +2661,7 @@ func (s *ManualCommitStrategy) finalizeAllTurnCheckpoints(ctx context.Context, s } store := checkpoint.NewGitStore(repo) + v2Only := settings.IsCheckpointsV2OnlyEnabled(logCtx) // Evaluate v2 flag once before the loop to avoid re-reading settings per checkpoint var v2Store *checkpoint.V2GitStore @@ -2691,8 +2692,17 @@ func (s *ManualCommitStrategy) finalizeAllTurnCheckpoints(ctx context.Context, s // Generate compact transcript for v2 /main if v2Store != nil && redactedTranscript.Len() > 0 { finalAg, _ := agent.GetByAgentType(state.AgentType) //nolint:errcheck // ag may be nil for unknown agent types; compactTranscriptForV2 handles nil + var ( + content *checkpoint.SessionContent + readErr error + ) + if v2Only { + content, readErr = v2Store.ReadSessionContentByID(ctx, cpID, state.SessionID) + } else { + content, readErr = store.ReadSessionContentByID(ctx, cpID, state.SessionID) + } startLine := 0 - if content, readErr := store.ReadSessionContentByID(ctx, cpID, state.SessionID); readErr == nil && content != nil { + if readErr == nil && content != nil { startLine = content.Metadata.GetTranscriptStart() } else { errMsg := "unknown" @@ -2708,23 +2718,30 @@ func (s *ManualCommitStrategy) finalizeAllTurnCheckpoints(ctx context.Context, s updateOpts.CompactTranscript = compactTranscriptForV2(logCtx, finalAg, redactedTranscript, startLine) } - updateErr := store.UpdateCommitted(ctx, updateOpts) - if updateErr != nil { - logging.Warn(logCtx, "finalize: failed to update checkpoint", - slog.String("checkpoint_id", cpIDStr), - slog.String("error", updateErr.Error()), - ) - errCount++ - continue + if !v2Only { + updateErr := store.UpdateCommitted(ctx, updateOpts) + if updateErr != nil { + logging.Warn(logCtx, "finalize: failed to update checkpoint", + slog.String("checkpoint_id", cpIDStr), + slog.String("error", updateErr.Error()), + ) + errCount++ + continue + } } - // Dual-write: update v2 refs when enabled if v2Store != nil { if v2Err := v2Store.UpdateCommitted(logCtx, updateOpts); v2Err != nil { - logging.Warn(logCtx, "v2 dual-write update failed", + attrs := []any{ slog.String("checkpoint_id", cpIDStr), slog.String("error", v2Err.Error()), - ) + } + if v2Only { + logging.Warn(logCtx, "finalize: failed to update checkpoint in v2", attrs...) + errCount++ + continue + } + logging.Warn(logCtx, "v2 dual-write update failed", attrs...) } } diff --git a/cmd/entire/cli/strategy/manual_commit_push.go b/cmd/entire/cli/strategy/manual_commit_push.go index 36c873f2b..c59595620 100644 --- a/cmd/entire/cli/strategy/manual_commit_push.go +++ b/cmd/entire/cli/strategy/manual_commit_push.go @@ -9,8 +9,10 @@ import ( ) // PrePush is called by the git pre-push hook before pushing to a remote. -// It pushes the entire/checkpoints/v1 branch alongside the user's push, -// and v2 refs when both checkpoints_v2 and push_v2_refs are enabled. +// It pushes the entire/checkpoints/v1 branch alongside the user's push (unless +// v1 writes are disabled by checkpoints_v2_only), and pushes v2 refs whenever +// IsPushV2RefsEnabled is true — i.e. either checkpoints_v2 + push_v2_refs, or +// checkpoints_v2_only. // // If a checkpoint_remote is configured in settings, checkpoint branches/refs // are pushed to the derived URL instead of the user's push remote. @@ -19,6 +21,7 @@ import ( // - push_sessions: false to disable automatic pushing of checkpoints // - checkpoint_remote: {"provider": "github", "repo": "org/repo"} to push to a separate repo // - push_v2_refs: true to enable pushing v2 refs (requires checkpoints_v2) +// - checkpoints_v2_only: true to skip the v1 metadata branch entirely and force v2 ref pushes on func (s *ManualCommitStrategy) PrePush(ctx context.Context, remote string) error { // Load settings once for remote resolution and push_sessions check ps := resolvePushSettings(ctx, remote) @@ -27,14 +30,17 @@ func (s *ManualCommitStrategy) PrePush(ctx context.Context, remote string) error return nil } - _, pushCheckpointsSpan := perf.Start(ctx, "push_checkpoints_branch") - err := pushBranchIfNeeded(ctx, ps.pushTarget(), paths.MetadataBranchName) - if err != nil { - pushCheckpointsSpan.RecordError(err) + var err error + if !settings.IsCheckpointsV2OnlyEnabled(ctx) { + _, pushCheckpointsSpan := perf.Start(ctx, "push_checkpoints_branch") + err = pushBranchIfNeeded(ctx, ps.pushTarget(), paths.MetadataBranchName) + if err != nil { + pushCheckpointsSpan.RecordError(err) + } + pushCheckpointsSpan.End() } - pushCheckpointsSpan.End() - // Push v2 refs when both checkpoints_v2 and push_v2_refs are enabled + // Push v2 refs when enabled. if settings.IsPushV2RefsEnabled(ctx) { _, pushV2Span := perf.Start(ctx, "push_v2_refs") pushV2Refs(ctx, ps.pushTarget()) diff --git a/cmd/entire/cli/strategy/push_common.go b/cmd/entire/cli/strategy/push_common.go index 889e203fd..387269c8d 100644 --- a/cmd/entire/cli/strategy/push_common.go +++ b/cmd/entire/cli/strategy/push_common.go @@ -11,6 +11,7 @@ import ( "sync" "time" + "github.com/entireio/cli/cmd/entire/cli/checkpoint" "github.com/entireio/cli/cmd/entire/cli/settings" "github.com/go-git/go-git/v6" @@ -120,6 +121,9 @@ func printCheckpointRemoteHint(target string) { // settingsHintOnce ensures the settings commit hint prints at most once per process. var settingsHintOnce sync.Once +// v2OnlyMigrationHintOnce ensures the v2-only migration hint prints at most once per process. +var v2OnlyMigrationHintOnce sync.Once + // printSettingsCommitHint prints a hint after a successful checkpoint remote push // when the committed .entire/settings.json does not contain a checkpoint_remote config. // entire.io discovers the external checkpoint repo by reading the committed project @@ -139,6 +143,53 @@ func printSettingsCommitHint(ctx context.Context, target string) { }) } +// printCheckpointsV2OnlyMigrationHint prints a hint when the committed project +// settings enable checkpoints_v2_only AND there are v1 checkpoints that have +// not yet been mirrored into v2. Suppressed when v2 already has every v1 +// checkpoint (nothing to migrate) so the hint does not become noise once the +// migration is done. +func printCheckpointsV2OnlyMigrationHint(ctx context.Context) { + v2OnlyMigrationHintOnce.Do(func() { + if !isCheckpointsV2OnlyCommitted(ctx) { + return + } + if !hasUnmigratedV1Checkpoints(ctx) { + return + } + fmt.Fprintln(os.Stderr, "[entire] Note: .entire/settings.json enables checkpoints_v2_only. Run 'entire migrate --checkpoints v2' to migrate existing checkpoints to v2.") + fmt.Fprintln(os.Stderr, "[entire] Use 'entire migrate --checkpoints v2 --force' to rewrite all checkpoints in v2.") + }) +} + +// hasUnmigratedV1Checkpoints reports whether any v1 checkpoint has no matching +// entry in v2. Any failure opening the repo or listing either store is treated +// as "no migration needed" so we stay silent instead of printing a speculative +// hint — the hint is advisory and should never be the reason a push gets noisy. +func hasUnmigratedV1Checkpoints(ctx context.Context) bool { + repo, err := OpenRepository(ctx) + if err != nil { + return false + } + v1List, err := checkpoint.NewGitStore(repo).ListCommitted(ctx) + if err != nil || len(v1List) == 0 { + return false + } + v2List, err := checkpoint.NewV2GitStore(repo, "").ListCommitted(ctx) + if err != nil { + return false + } + v2Set := make(map[string]struct{}, len(v2List)) + for _, info := range v2List { + v2Set[info.CheckpointID.String()] = struct{}{} + } + for _, info := range v1List { + if _, ok := v2Set[info.CheckpointID.String()]; !ok { + return true + } + } + return false +} + // isCheckpointRemoteCommitted returns true if the committed .entire/settings.json // at HEAD contains a valid checkpoint_remote configuration. This is the true // discoverability check: entire.io reads from committed project settings, not from @@ -157,6 +208,21 @@ func isCheckpointRemoteCommitted(ctx context.Context) bool { return committed.GetCheckpointRemote() != nil } +// isCheckpointsV2OnlyCommitted returns true if the committed .entire/settings.json +// at HEAD enables checkpoints_v2_only. +func isCheckpointsV2OnlyCommitted(ctx context.Context) bool { + cmd := exec.CommandContext(ctx, "git", "show", "HEAD:.entire/settings.json") + output, err := cmd.Output() + if err != nil { + return false + } + committed, err := settings.LoadFromBytes(output) + if err != nil { + return false + } + return committed.IsCheckpointsV2OnlyEnabled() +} + // pushResult describes what happened during a push attempt. type pushResult struct { // upToDate is true when the remote already had all commits (nothing transferred). @@ -189,6 +255,7 @@ func finishPush(ctx context.Context, stop func(string), result pushResult, targe stop(" done") printSettingsCommitHint(ctx, target) } + printCheckpointsV2OnlyMigrationHint(ctx) } // tryPushSessionsCommon attempts to push the sessions branch. diff --git a/cmd/entire/cli/strategy/push_common_test.go b/cmd/entire/cli/strategy/push_common_test.go index 5c132ae26..ae8c279a2 100644 --- a/cmd/entire/cli/strategy/push_common_test.go +++ b/cmd/entire/cli/strategy/push_common_test.go @@ -6,12 +6,15 @@ import ( "os" "os/exec" "path/filepath" + "strings" "sync" "testing" "github.com/entireio/cli/cmd/entire/cli/checkpoint" + "github.com/entireio/cli/cmd/entire/cli/checkpoint/id" "github.com/entireio/cli/cmd/entire/cli/paths" "github.com/entireio/cli/cmd/entire/cli/testutil" + "github.com/entireio/cli/redact" "github.com/go-git/go-git/v6" "github.com/go-git/go-git/v6/plumbing" @@ -1203,6 +1206,167 @@ func TestPrintSettingsCommitHint(t *testing.T) { }) } +func TestIsCheckpointsV2OnlyCommitted(t *testing.T) { + t.Run("false when settings.json not committed", func(t *testing.T) { + tmpDir := t.TempDir() + testutil.InitRepo(t, tmpDir) + testutil.WriteFile(t, tmpDir, "f.txt", "init") + testutil.GitAdd(t, tmpDir, "f.txt") + testutil.GitCommit(t, tmpDir, "init") + + entireDir := filepath.Join(tmpDir, ".entire") + require.NoError(t, os.MkdirAll(entireDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(entireDir, "settings.json"), + []byte(`{"strategy_options":{"checkpoints_v2_only":true}}`), 0o644)) + + t.Chdir(tmpDir) + assert.False(t, isCheckpointsV2OnlyCommitted(context.Background())) + }) + + t.Run("true when checkpoints_v2_only is committed", func(t *testing.T) { + tmpDir := t.TempDir() + testutil.InitRepo(t, tmpDir) + testutil.WriteFile(t, tmpDir, "f.txt", "init") + testutil.GitAdd(t, tmpDir, "f.txt") + testutil.GitCommit(t, tmpDir, "init") + + entireDir := filepath.Join(tmpDir, ".entire") + require.NoError(t, os.MkdirAll(entireDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(entireDir, "settings.json"), + []byte(`{"strategy_options":{"checkpoints_v2_only":true}}`), 0o644)) + testutil.GitAdd(t, tmpDir, ".entire/settings.json") + testutil.GitCommit(t, tmpDir, "enable checkpoints_v2_only") + + t.Chdir(tmpDir) + assert.True(t, isCheckpointsV2OnlyCommitted(context.Background())) + }) +} + +// setupV2OnlyCommittedRepo creates a temp repo with checkpoints_v2_only enabled +// in the committed .entire/settings.json and chdirs into it. Returns an opened +// *git.Repository for populating checkpoints. +func setupV2OnlyCommittedRepo(t *testing.T) *git.Repository { + t.Helper() + + tmpDir := t.TempDir() + testutil.InitRepo(t, tmpDir) + testutil.WriteFile(t, tmpDir, "f.txt", "init") + testutil.GitAdd(t, tmpDir, "f.txt") + testutil.GitCommit(t, tmpDir, "init") + + entireDir := filepath.Join(tmpDir, ".entire") + require.NoError(t, os.MkdirAll(entireDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(entireDir, "settings.json"), + []byte(`{"strategy_options":{"checkpoints_v2_only":true}}`), 0o644)) + testutil.GitAdd(t, tmpDir, ".entire/settings.json") + testutil.GitCommit(t, tmpDir, "enable checkpoints_v2_only") + t.Chdir(tmpDir) + + repo, err := git.PlainOpen(tmpDir) + require.NoError(t, err) + return repo +} + +// writeV1Checkpoint writes a minimal checkpoint to the v1 metadata branch. +func writeV1Checkpoint(t *testing.T, repo *git.Repository, cpID id.CheckpointID, sessionID string) { + t.Helper() + err := checkpoint.NewGitStore(repo).WriteCommitted(context.Background(), checkpoint.WriteCommittedOptions{ + CheckpointID: cpID, + SessionID: sessionID, + Strategy: "manual-commit", + Transcript: redact.AlreadyRedacted([]byte(`{"from":"` + sessionID + `"}`)), + AuthorName: "Test", + AuthorEmail: "test@test.com", + }) + require.NoError(t, err) +} + +func TestPrintCheckpointsV2OnlyMigrationHint(t *testing.T) { + t.Run("suppressed when no v1 checkpoints exist", func(t *testing.T) { + v2OnlyMigrationHintOnce = sync.Once{} + setupV2OnlyCommittedRepo(t) + + restore := captureStderr(t) + printCheckpointsV2OnlyMigrationHint(context.Background()) + output := restore() + + assert.Empty(t, output, "hint should not print when there are no v1 checkpoints to migrate") + }) + + t.Run("suppressed when every v1 checkpoint is already in v2", func(t *testing.T) { + v2OnlyMigrationHintOnce = sync.Once{} + repo := setupV2OnlyCommittedRepo(t) + + cpID := id.MustCheckpointID("aabbccddeeff") + writeV1Checkpoint(t, repo, cpID, "session-1") + writeV2Checkpoint(t, repo, cpID, "session-1") + + restore := captureStderr(t) + printCheckpointsV2OnlyMigrationHint(context.Background()) + output := restore() + + assert.Empty(t, output, "hint should not print once v2 already mirrors every v1 checkpoint") + }) + + t.Run("prints when v1 has checkpoints not in v2", func(t *testing.T) { + v2OnlyMigrationHintOnce = sync.Once{} + repo := setupV2OnlyCommittedRepo(t) + + writeV1Checkpoint(t, repo, id.MustCheckpointID("111111111111"), "session-1") + + restore := captureStderr(t) + printCheckpointsV2OnlyMigrationHint(context.Background()) + output := restore() + + assert.Contains(t, output, "entire migrate --checkpoints v2") + assert.Contains(t, output, "entire migrate --checkpoints v2 --force") + }) + + t.Run("prints only once per process", func(t *testing.T) { + v2OnlyMigrationHintOnce = sync.Once{} + repo := setupV2OnlyCommittedRepo(t) + + writeV1Checkpoint(t, repo, id.MustCheckpointID("222222222222"), "session-2") + + restore := captureStderr(t) + printCheckpointsV2OnlyMigrationHint(context.Background()) + printCheckpointsV2OnlyMigrationHint(context.Background()) + output := restore() + + // --force appears in exactly one line, so its count equals the number of + // invocations that actually emitted output. + forceCount := strings.Count(output, "--force") + assert.Equal(t, 1, forceCount, "hint should print exactly once per process") + }) +} + +func TestHasUnmigratedV1Checkpoints(t *testing.T) { + t.Run("false when no v1 checkpoints exist", func(t *testing.T) { + setupV2OnlyCommittedRepo(t) + assert.False(t, hasUnmigratedV1Checkpoints(context.Background())) + }) + + t.Run("false when every v1 checkpoint is in v2", func(t *testing.T) { + repo := setupV2OnlyCommittedRepo(t) + cpID := id.MustCheckpointID("333333333333") + writeV1Checkpoint(t, repo, cpID, "session-a") + writeV2Checkpoint(t, repo, cpID, "session-a") + + assert.False(t, hasUnmigratedV1Checkpoints(context.Background())) + }) + + t.Run("true when at least one v1 checkpoint is missing from v2", func(t *testing.T) { + repo := setupV2OnlyCommittedRepo(t) + mirrored := id.MustCheckpointID("444444444444") + missing := id.MustCheckpointID("555555555555") + writeV1Checkpoint(t, repo, mirrored, "session-b") + writeV2Checkpoint(t, repo, mirrored, "session-b") + writeV1Checkpoint(t, repo, missing, "session-c") + + assert.True(t, hasUnmigratedV1Checkpoints(context.Background())) + }) +} + // captureStderr redirects os.Stderr to a pipe and returns a function that restores // stderr and returns the captured output. Must be called on the main goroutine // (not parallel-safe). Uses t.Cleanup as a safety net to restore stderr and close diff --git a/cmd/entire/cli/summarize/summarize_test.go b/cmd/entire/cli/summarize/summarize_test.go index 0a0d326f1..80afe6b9e 100644 --- a/cmd/entire/cli/summarize/summarize_test.go +++ b/cmd/entire/cli/summarize/summarize_test.go @@ -827,9 +827,7 @@ func TestBuildCondensedTranscriptFromBytes_Codex_ExecCommandDetail(t *testing.T) break } } - if toolEntry == nil { - t.Fatalf("no tool entry found in entries: %#v", entries) - } + require.NotNil(t, toolEntry, "no tool entry found in entries: %#v", entries) if toolEntry.ToolName != "exec_command" { t.Fatalf("expected exec_command, got %q", toolEntry.ToolName) }