Skip to content

Commit 095cc82

Browse files
authored
Merge pull request #1001 from entireio/update-checkpoints-version-setting
Checkpoints V2: allow for a `checkpoints_version` setting in settings.json
2 parents b5e3167 + 2e9e766 commit 095cc82

11 files changed

Lines changed: 163 additions & 116 deletions

cmd/entire/cli/attach.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -177,17 +177,18 @@ func runAttach(ctx context.Context, w io.Writer, sessionID string, agentName typ
177177
writeOpts.CompactTranscript = compacted
178178
}
179179

180-
v2Only := settings.IsCheckpointsV2OnlyEnabled(logCtx)
181-
if !v2Only {
180+
v2 := settings.CheckpointsVersion(logCtx) == 2
181+
if !v2 {
182182
if err := store.WriteCommitted(ctx, writeOpts); err != nil {
183183
return fmt.Errorf("failed to write checkpoint: %w", err)
184184
}
185185
}
186-
// IsCheckpointsV2Enabled is true whenever v2Only is true, so this covers both
187-
// the v2-only and dual-write paths. Only v2-only propagates the error.
186+
// IsCheckpointsV2Enabled is true whenever v2 writes are enabled, including
187+
// both v2-only mode (checkpoints_version == 2) and dual-write mode. Only
188+
// v2-only mode propagates the error.
188189
if settings.IsCheckpointsV2Enabled(logCtx) {
189190
if err := writeAttachCheckpointV2(logCtx, repo, writeOpts); err != nil {
190-
if v2Only {
191+
if v2 {
191192
return fmt.Errorf("failed to write checkpoint to v2: %w", err)
192193
}
193194
logging.Warn(logCtx, "attach v2 dual-write failed", "error", err)

cmd/entire/cli/integration_test/v2_dual_write_test.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -245,10 +245,11 @@ func TestV2DualWrite_StopTimeFinalization(t *testing.T) {
245245
assert.Contains(t, compactTranscript, `"v":1`)
246246
}
247247

248-
// TestV2Only_SkipsV1Write verifies the v2-only specific deltas: v1 metadata is
249-
// not written and v2 refs still exist. The full v2 payload shape is already
250-
// covered by TestV2DualWrite_FullWorkflow.
251-
func TestV2Only_SkipsV1Write(t *testing.T) {
248+
// TestCheckpointsVersion2_SkipsV1Write verifies the specific deltas of running
249+
// with checkpoints_version: 2 — v1 metadata is not written and v2 refs still
250+
// exist. The full v2 payload shape is already covered by
251+
// TestV2DualWrite_FullWorkflow.
252+
func TestCheckpointsVersion2_SkipsV1Write(t *testing.T) {
252253
t.Parallel()
253254
env := NewTestEnv(t)
254255
defer env.Cleanup()
@@ -259,10 +260,10 @@ func TestV2Only_SkipsV1Write(t *testing.T) {
259260
env.GitAdd("README.md")
260261
env.GitAdd(".gitignore")
261262
env.GitCommit("Initial commit")
262-
env.GitCheckoutNewBranch("feature/v2-only-test")
263+
env.GitCheckoutNewBranch("feature/checkpoints-v2-test")
263264

264265
env.InitEntireWithOptions(map[string]any{
265-
"checkpoints_v2_only": true,
266+
"checkpoints_version": 2,
266267
})
267268

268269
session := env.NewSession()
@@ -287,7 +288,7 @@ func TestV2Only_SkipsV1Write(t *testing.T) {
287288
// v1: should NOT be written.
288289
_, found := env.ReadFileFromBranch(paths.MetadataBranchName, cpPath+"/"+paths.MetadataFileName)
289290
assert.False(t, found,
290-
"v1 committed checkpoint metadata should NOT exist when checkpoints_v2_only is enabled")
291+
"v1 committed checkpoint metadata should NOT exist when checkpoints_version is 2")
291292

292293
// v2: smoke check that the checkpoint still landed.
293294
assert.True(t, env.RefExists(paths.V2MainRefName), "v2 /main ref should exist")

cmd/entire/cli/integration_test/v2_push_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -124,10 +124,10 @@ func TestV2Push_Disabled_NoV2Refs(t *testing.T) {
124124
"v1 metadata branch should still exist on remote")
125125
}
126126

127-
// TestV2Push_V2OnlySkipsV1Branch verifies that the v1 metadata branch is not
128-
// pushed when checkpoints_v2_only is enabled; v2 ref pushing itself is covered
127+
// TestV2Push_Version2SkipsV1Branch verifies that the v1 metadata branch is not
128+
// pushed when checkpoints_version is set to 2; v2 ref pushing itself is covered
129129
// by TestV2Push_FullCycle.
130-
func TestV2Push_V2OnlySkipsV1Branch(t *testing.T) {
130+
func TestV2Push_Version2SkipsV1Branch(t *testing.T) {
131131
t.Parallel()
132132
env := NewTestEnv(t)
133133
defer env.Cleanup()
@@ -138,10 +138,10 @@ func TestV2Push_V2OnlySkipsV1Branch(t *testing.T) {
138138
env.GitAdd("README.md")
139139
env.GitAdd(".gitignore")
140140
env.GitCommit("Initial commit")
141-
env.GitCheckoutNewBranch("feature/v2-only-push-test")
141+
env.GitCheckoutNewBranch("feature/checkpoints-v2-push-test")
142142

143143
env.InitEntireWithOptions(map[string]any{
144-
"checkpoints_v2_only": true,
144+
"checkpoints_version": 2,
145145
})
146146

147147
bareDir := env.SetupBareRemote()
@@ -162,7 +162,7 @@ func TestV2Push_V2OnlySkipsV1Branch(t *testing.T) {
162162
env.RunPrePush("origin")
163163

164164
assert.False(t, bareRefExists(t, bareDir, "refs/heads/"+paths.MetadataBranchName),
165-
"v1 metadata branch should NOT exist on remote when checkpoints_v2_only is enabled")
165+
"v1 metadata branch should NOT exist on remote when checkpoints_version is 2")
166166
// Smoke: v2 refs still land; full payload asserted in TestV2Push_FullCycle.
167167
assert.True(t, bareRefExists(t, bareDir, paths.V2MainRefName),
168168
"v2 /main ref should exist on remote after push")

cmd/entire/cli/settings/settings.go

Lines changed: 59 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ import (
1010
"fmt"
1111
"os"
1212
"path/filepath"
13+
"strconv"
1314
"strings"
15+
"sync"
1416
"time"
1517

1618
"github.com/entireio/cli/cmd/entire/cli/jsonutil"
@@ -27,6 +29,8 @@ const (
2729
defaultGenerationRetentionDays = 60
2830
)
2931

32+
var checkpointsVersionWarningOnce sync.Once
33+
3034
// Commit linking mode constants.
3135
const (
3236
// CommitLinkingAlways auto-links commits to sessions without prompting.
@@ -607,14 +611,27 @@ func IsCheckpointsV2Enabled(ctx context.Context) bool {
607611
return settings.IsCheckpointsV2Enabled()
608612
}
609613

610-
// IsCheckpointsV2OnlyEnabled checks if checkpoints should be written and pushed
611-
// only via v2 refs.
612-
func IsCheckpointsV2OnlyEnabled(ctx context.Context) bool {
614+
// CheckpointsVersion returns the configured checkpoints format version, or 1
615+
// if settings cannot be loaded or the value is unset/invalid.
616+
func CheckpointsVersion(ctx context.Context) int {
613617
s, err := Load(ctx)
614618
if err != nil {
615-
return false
619+
return 1
620+
}
621+
version := s.CheckpointsVersion()
622+
if s.StrategyOptions != nil {
623+
if configured, ok := s.StrategyOptions["checkpoints_version"]; ok {
624+
if _, supported := parseCheckpointsVersion(configured); !supported {
625+
checkpointsVersionWarningOnce.Do(func() {
626+
fmt.Fprintf(os.Stderr,
627+
"[entire] unsupported strategy_options.checkpoints_version %v detected in settings. Falling back to the default version (1).\n",
628+
configured,
629+
)
630+
})
631+
}
632+
}
616633
}
617-
return s.IsCheckpointsV2OnlyEnabled()
634+
return version
618635
}
619636

620637
// IsPushV2RefsEnabled checks if pushing v2 refs is enabled in settings.
@@ -709,9 +726,9 @@ func (s *EntireSettings) GetCheckpointRemote() *CheckpointRemoteConfig {
709726
}
710727

711728
// IsCheckpointsV2Enabled checks if checkpoints v2 is enabled.
712-
// Returns true when either checkpoints_v2 or checkpoints_v2_only is enabled.
729+
// Returns true when either checkpoints_v2 is set or checkpoints_version is 2.
713730
func (s *EntireSettings) IsCheckpointsV2Enabled() bool {
714-
if s.IsCheckpointsV2OnlyEnabled() {
731+
if s.CheckpointsVersion() == 2 {
715732
return true
716733
}
717734
if s.StrategyOptions == nil {
@@ -721,20 +738,47 @@ func (s *EntireSettings) IsCheckpointsV2Enabled() bool {
721738
return ok && val
722739
}
723740

724-
// IsCheckpointsV2OnlyEnabled checks if checkpoints should be written and pushed
725-
// only via v2 refs, with no v1 dual-write.
726-
func (s *EntireSettings) IsCheckpointsV2OnlyEnabled() bool {
741+
// CheckpointsVersion returns the configured checkpoints format version from
742+
// strategy_options.checkpoints_version. Returns 1 when unset, invalid, or
743+
// unsupported. The currently supported versions are 1 and 2.
744+
func (s *EntireSettings) CheckpointsVersion() int {
727745
if s.StrategyOptions == nil {
728-
return false
746+
return 1
729747
}
730-
val, ok := s.StrategyOptions["checkpoints_v2_only"].(bool)
731-
return ok && val
748+
val, ok := s.StrategyOptions["checkpoints_version"]
749+
if !ok {
750+
return 1
751+
}
752+
version, ok := parseCheckpointsVersion(val)
753+
if ok {
754+
return version
755+
}
756+
return 1
757+
}
758+
759+
func parseCheckpointsVersion(val any) (int, bool) {
760+
v, ok := val.(int)
761+
if ok && (v == 1 || v == 2) {
762+
return v, true
763+
}
764+
floatV, ok := val.(float64)
765+
if ok && (floatV == 1 || floatV == 2) {
766+
return int(floatV), true
767+
}
768+
stringV, ok := val.(string)
769+
if ok {
770+
parsed, err := strconv.Atoi(stringV)
771+
if err == nil && (parsed == 1 || parsed == 2) {
772+
return parsed, true
773+
}
774+
}
775+
return 1, false
732776
}
733777

734778
// IsPushV2RefsEnabled checks if pushing v2 refs is enabled.
735-
// checkpoints_v2_only forces v2 ref pushes on, regardless of push_v2_refs.
779+
// checkpoints_version: 2 forces v2 ref pushes on, regardless of push_v2_refs.
736780
func (s *EntireSettings) IsPushV2RefsEnabled() bool {
737-
if s.IsCheckpointsV2OnlyEnabled() {
781+
if s.CheckpointsVersion() == 2 {
738782
return true
739783
}
740784
if !s.IsCheckpointsV2Enabled() {

cmd/entire/cli/settings/settings_test.go

Lines changed: 28 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -692,14 +692,14 @@ func TestIsCheckpointsV2Enabled_True(t *testing.T) {
692692
}
693693
}
694694

695-
func TestIsCheckpointsV2Enabled_V2Only(t *testing.T) {
695+
func TestIsCheckpointsV2Enabled_CheckpointsVersion2(t *testing.T) {
696696
t.Parallel()
697697
s := &EntireSettings{
698698
Enabled: true,
699-
StrategyOptions: map[string]any{"checkpoints_v2_only": true},
699+
StrategyOptions: map[string]any{"checkpoints_version": 2},
700700
}
701701
if !s.IsCheckpointsV2Enabled() {
702-
t.Error("expected IsCheckpointsV2Enabled to be true when checkpoints_v2_only is enabled")
702+
t.Error("expected IsCheckpointsV2Enabled to be true when checkpoints_version is 2")
703703
}
704704
}
705705

@@ -788,33 +788,34 @@ func TestIsCheckpointsV2Enabled_LocalOverride(t *testing.T) {
788788
}
789789
}
790790

791-
func TestIsCheckpointsV2OnlyEnabled_DefaultsFalse(t *testing.T) {
791+
func TestCheckpointsVersion(t *testing.T) {
792792
t.Parallel()
793-
s := &EntireSettings{Enabled: true}
794-
if s.IsCheckpointsV2OnlyEnabled() {
795-
t.Error("expected IsCheckpointsV2OnlyEnabled to default to false")
796-
}
797-
}
798793

799-
func TestIsCheckpointsV2OnlyEnabled_True(t *testing.T) {
800-
t.Parallel()
801-
s := &EntireSettings{
802-
Enabled: true,
803-
StrategyOptions: map[string]any{"checkpoints_v2_only": true},
804-
}
805-
if !s.IsCheckpointsV2OnlyEnabled() {
806-
t.Error("expected IsCheckpointsV2OnlyEnabled to be true")
794+
tests := []struct {
795+
name string
796+
opts map[string]any
797+
want int
798+
}{
799+
{"unset defaults to one", nil, 1},
800+
{"empty options defaults to one", map[string]any{}, 1},
801+
{"integer 2", map[string]any{"checkpoints_version": 2}, 2},
802+
{"float 2 from json", map[string]any{"checkpoints_version": float64(2)}, 2},
803+
{"integer 3 falls back to default", map[string]any{"checkpoints_version": 3}, 1},
804+
{"zero falls back to default", map[string]any{"checkpoints_version": 0}, 1},
805+
{"negative falls back to default", map[string]any{"checkpoints_version": -1}, 1},
806+
{"non-integer float falls back to default", map[string]any{"checkpoints_version": 2.5}, 1},
807+
{"string 2", map[string]any{"checkpoints_version": "2"}, 2},
808+
{"bool falls back to default", map[string]any{"checkpoints_version": true}, 1},
807809
}
808-
}
809810

810-
func TestIsCheckpointsV2OnlyEnabled_WrongType(t *testing.T) {
811-
t.Parallel()
812-
s := &EntireSettings{
813-
Enabled: true,
814-
StrategyOptions: map[string]any{"checkpoints_v2_only": "yes"},
815-
}
816-
if s.IsCheckpointsV2OnlyEnabled() {
817-
t.Error("expected IsCheckpointsV2OnlyEnabled to be false for non-bool value")
811+
for _, tt := range tests {
812+
t.Run(tt.name, func(t *testing.T) {
813+
t.Parallel()
814+
s := &EntireSettings{StrategyOptions: tt.opts}
815+
if got := s.CheckpointsVersion(); got != tt.want {
816+
t.Errorf("CheckpointsVersion() = %d, want %d", got, tt.want)
817+
}
818+
})
818819
}
819820
}
820821

@@ -834,7 +835,7 @@ func TestIsPushV2RefsEnabled_RequiresBothFlags(t *testing.T) {
834835
opts map[string]any
835836
expected bool
836837
}{
837-
{"v2 only supersedes both", map[string]any{"checkpoints_v2": false, "push_v2_refs": false, "checkpoints_v2_only": true}, true},
838+
{"checkpoints_version 2 supersedes both", map[string]any{"checkpoints_v2": false, "push_v2_refs": false, "checkpoints_version": 2}, true},
838839
{"both true", map[string]any{"checkpoints_v2": true, "push_v2_refs": true}, true},
839840
{"only checkpoints_v2", map[string]any{"checkpoints_v2": true}, false},
840841
{"only push_v2_refs", map[string]any{"push_v2_refs": true}, false},

cmd/entire/cli/strategy/checkpoint_remote.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,9 @@ func resolvePushSettings(ctx context.Context, pushRemoteName string) pushSetting
8383

8484
ps.checkpointURL = checkpointURL
8585

86-
// Skip the v1 metadata-branch fetch entirely in v2-only mode — there is no
87-
// v1 branch being written or pushed, so there is nothing to sync.
88-
if !s.IsCheckpointsV2OnlyEnabled() {
86+
// Skip the v1 metadata-branch fetch entirely when checkpoints_version is 2 —
87+
// there is no v1 branch being written or pushed, so there is nothing to sync.
88+
if s.CheckpointsVersion() != 2 {
8989
// If the v1 checkpoint branch doesn't exist locally, try to fetch it from the URL.
9090
// This is a one-time operation — once the branch exists locally, subsequent pushes
9191
// skip the fetch entirely. Only fetch the metadata branch; trails are always pushed

cmd/entire/cli/strategy/manual_commit_condensation.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -254,12 +254,12 @@ func (s *ManualCommitStrategy) CondenseSession(ctx context.Context, repo *git.Re
254254

255255
compactTranscriptDuration := buildCompactTranscript(ctx, ag, redactedTranscript, state, &writeOpts)
256256

257-
v2Only := settings.IsCheckpointsV2OnlyEnabled(ctx)
257+
v2 := settings.CheckpointsVersion(ctx) == 2
258258

259259
// Write checkpoint metadata to the primary store.
260260
writeV1Start := time.Now()
261261
writeCtx, writeCommittedSpan := perf.Start(ctx, "write_committed_v1")
262-
if !v2Only {
262+
if !v2 {
263263
if err := store.WriteCommitted(writeCtx, writeOpts); err != nil {
264264
writeCommittedSpan.RecordError(err)
265265
writeCommittedSpan.End()
@@ -271,7 +271,7 @@ func (s *ManualCommitStrategy) CondenseSession(ctx context.Context, repo *git.Re
271271

272272
writeV2Start := time.Now()
273273
writeV2Ctx, writeCommittedV2Span := perf.Start(ctx, "write_committed_v2")
274-
if v2Only {
274+
if v2 {
275275
if err := writeCommittedV2(writeV2Ctx, repo, writeOpts); err != nil {
276276
writeCommittedV2Span.RecordError(err)
277277
writeCommittedV2Span.End()

cmd/entire/cli/strategy/manual_commit_hooks.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2745,7 +2745,7 @@ func (s *ManualCommitStrategy) finalizeAllTurnCheckpoints(ctx context.Context, s
27452745
}
27462746

27472747
store := checkpoint.NewGitStore(repo)
2748-
v2Only := settings.IsCheckpointsV2OnlyEnabled(logCtx)
2748+
v2 := settings.CheckpointsVersion(logCtx) == 2
27492749

27502750
// Evaluate v2 flag once before the loop to avoid re-reading settings per checkpoint
27512751
var v2Store *checkpoint.V2GitStore
@@ -2790,7 +2790,7 @@ func (s *ManualCommitStrategy) finalizeAllTurnCheckpoints(ctx context.Context, s
27902790
content *checkpoint.SessionContent
27912791
readErr error
27922792
)
2793-
if v2Only {
2793+
if v2 {
27942794
content, readErr = v2Store.ReadSessionContentByID(ctx, cpID, state.SessionID)
27952795
} else {
27962796
content, readErr = store.ReadSessionContentByID(ctx, cpID, state.SessionID)
@@ -2812,7 +2812,7 @@ func (s *ManualCommitStrategy) finalizeAllTurnCheckpoints(ctx context.Context, s
28122812
updateOpts.CompactTranscript = compactTranscriptForV2(logCtx, finalAg, redactedTranscript, startLine)
28132813
}
28142814

2815-
if !v2Only {
2815+
if !v2 {
28162816
updateErr := store.UpdateCommitted(ctx, updateOpts)
28172817
if updateErr != nil {
28182818
logging.Warn(logCtx, "finalize: failed to update checkpoint",
@@ -2830,7 +2830,7 @@ func (s *ManualCommitStrategy) finalizeAllTurnCheckpoints(ctx context.Context, s
28302830
slog.String("checkpoint_id", cpIDStr),
28312831
slog.String("error", v2Err.Error()),
28322832
}
2833-
if v2Only {
2833+
if v2 {
28342834
logging.Warn(logCtx, "finalize: failed to update checkpoint in v2", attrs...)
28352835
errCount++
28362836
continue

0 commit comments

Comments
 (0)