Address Checkpoint Policy Review Feedback

aabfb16·

pfleidi·3w ago·6 files·+153 added/-11 removed

Keep resume usable when a squash commit lists an older unsupported checkpoint before a readable one, while preserving unsupported-version failures when no readable checkpoint remains.

Silence policy command cancellation and use the safe update instruction for checkpoint policy upgrade warnings.

Sessions

8a621d57590dView transcript

?\
Implement Checkpoint Policy Management SystemCodex·GPT-5.5·5 steps

Changes

6

• cmd/entire/cli

• Mpolicy_checkpoint.go+18/-5

  • Mpolicy_checkpoint_test.go+25
  • Mresume.go+8/-1
  • Mresume_test.go+53
  • versioncheck

• Mversioncheck.go+3

  • Mversioncheck_test.go+46/-5

1
2
3
4
5
6
7
8
26 unmodified lines

35
36
37
38
39
40
41
42
38
43
44
45
46
47
48
44
49
50
51
52
4 unmodified lines

57
58
59
55
60
61
62
58
63
64
65
66
67
68
64
69
70
71
72
6 unmodified lines

79
80
81
82
83
84
85
86
87
88
89

package cli

import (
    "context"
    "errors"
    "fmt"

"github.com/entireio/cli/cmd/entire/cli/checkpointpolicy"
)

func runPolicyCheckpoint(cmd *cobra.Command, opts policyCheckpointOptions) error {
    ctx := cmd.Context()
    if err := ctx.Err(); err != nil {
        return NewSilentError(err)
    }
    repo, err := gitrepo.OpenCurrent(ctx)
    if err != nil {
        return fmt.Errorf("open repository: %w", err)
        return policyCheckpointError("open repository", err)
    }
    defer repo.Close()

target, err := checkpointpolicy.ResolveTarget(ctx)
    if err != nil {
        return fmt.Errorf("resolve checkpoint policy remote: %w", err)
        return policyCheckpointError("resolve checkpoint policy remote", err)
    }

var state checkpointpolicy.State

Force:                opts.force,
    })
    if err != nil {
        return fmt.Errorf("update checkpoint policy: %w", err)
        return policyCheckpointError("update checkpoint policy", err)
    }
    if err := checkpointpolicy.Push(ctx, target); err != nil {
        return fmt.Errorf("push checkpoint policy: %w", err)
        return policyCheckpointError("push checkpoint policy", err)
    }
    state.Source = checkpointpolicy.SourceRemote
    } else {
        state, err = checkpointpolicy.Sync(ctx, repo, target)
        if err != nil {
            return fmt.Errorf("sync checkpoint policy: %w", err)
            return policyCheckpointError("sync checkpoint policy", err)
        }
    }

func hasPolicyCheckpointUpdate(opts policyCheckpointOptions) bool {
    return opts.version != "" || opts.minVersion != ""
}

func policyCheckpointError(message string, err error) error {
    wrapped := fmt.Errorf("%s: %w", message, err)
    if errors.Is(wrapped, context.Canceled) {
        return NewSilentError(wrapped)
    }
    return wrapped
}

Mcmd/entire/cli/policy_checkpoint.go+18/-5

2 unmodified lines

3
4
5
6
7
8
9
3 unmodified lines

13
14
15
16
17
18
19
63 unmodified lines

83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111

2 unmodified lines

import (
    "bytes"
    "context"
    "fmt"
    "os/exec"
    "path/filepath"
    "strings"

"github.com/entireio/cli/cmd/entire/cli/testutil"
    "github.com/go-git/go-git/v6"
    "github.com/go-git/go-git/v6/plumbing"
    "github.com/spf13/cobra"
    "github.com/stretchr/testify/require"
)

63 unmodified lines

require.Empty(t, strings.TrimSpace(branches))
}

func TestPolicyCheckpointCmd_SilencesContextCanceled(t *testing.T) {
    cmd := &cobra.Command{}
    var stdout, stderr bytes.Buffer
    cmd.SetOut(&stdout)
    cmd.SetErr(&stderr)
    ctx, cancel := context.WithCancel(context.Background())
    cancel()
    cmd.SetContext(ctx)

err := runPolicyCheckpoint(cmd, policyCheckpointOptions{})
    require.ErrorIs(t, err, context.Canceled)
    var silent *SilentError
    require.ErrorAs(t, err, &silent, "error = %T %v, want SilentError", err, err)
    require.Empty(t, stderr.String())
}

func TestPolicyCheckpointErrorSilencesWrappedContextCanceled(t *testing.T) {
    err := policyCheckpointError("sync checkpoint policy", fmt.Errorf("remote: %%w", context.Canceled))
    require.ErrorIs(t, err, context.Canceled)
    var silent *SilentError
    require.ErrorAs(t, err, &silent, "error = %T %v, want SilentError", err, err)
}

func setupPolicyCheckpointRepo(t *testing.T) (string, string) {
    t.Helper()
    testutil.IsolateGitConfigEnv(t)

Mcmd/entire/cli/policy_checkpoint_test.go+25

339 unmodified lines

340
341
342
343
344
345
346
347
347
348
349
350
351
352
353
354
5 unmodified lines

360
361
362
363
364
365
366
367
368

339 unmodified lines

// the checkpoint with the latest CreatedAt.
func resolveLatestCheckpoint(ctx context.Context, store checkpointInfoReader, checkpointIDs []id.CheckpointID) (*strategy.CheckpointInfo, bool, error) {
    infoMap := make(map[id.CheckpointID]strategy.CheckpointInfo, len(checkpointIDs))
    var unsupportedErr error
    for _, cpID := range checkpointIDs {
        metadata, readErr := readCheckpointInfoFromStore(ctx, store, cpID)
        if readErr != nil {
            if checkpointpolicy.IsUnsupportedVersion(readErr) {
                return nil, false, readErr
                if unsupportedErr == nil {
                    unsupportedErr = readErr
                }
                continue
            }
            logging.Debug(ctx, "resolveLatestCheckpoint: checkpoint metadata read failed",
                slog.String("checkpoint_id", cpID.String()),
            5 unmodified lines

}
    latest, found := strategy.ResolveLatestCheckpointFromMap(checkpointIDs, infoMap)
    if !found {
        if unsupportedErr != nil {
            return nil, false, unsupportedErr
        }
        return nil, false, nil
    }
    return &latest, true, nil
}

Mcmd/entire/cli/resume.go+8/-1

14 unmodified lines

15
16
17
18
19
20
21
611 unmodified lines

633
634
635
636
637
638
639
640
641
642
643
644
645
646
647
648
649
650
651
652
653
654
655
656
657
658
659
660
661
662
663
664
665
666
667
668
669
670
671
672
673
674
675
676
677
678
679
680
681
682
683
684
685
686
687
688
689
690

14 unmodified lines

"github.com/entireio/cli/cmd/entire/cli/agent/types"
    "github.com/entireio/cli/cmd/entire/cli/checkpoint"
    "github.com/entireio/cli/cmd/entire/cli/checkpoint/id"
    "github.com/entireio/cli/cmd/entire/cli/checkpointpolicy"
    "github.com/entireio/cli/cmd/entire/cli/paths"
    "github.com/entireio/cli/cmd/entire/cli/strategy"
    "github.com/entireio/cli/cmd/entire/cli/testutil"
611 unmodified lines

}

func TestResolveLatestCheckpointSkipsUnsupportedCheckpointWhenReadableCheckpointExists(t *testing.T) {
    t.Parallel()

unsupportedID := id.MustCheckpointID("aaa111bbb222")
    newID := id.MustCheckpointID("ccc333ddd444")
    reader := &resumeCheckpointInfoReaderStub{
        summaries: map[id.CheckpointID]*checkpoint.CheckpointSummary{
            unsupportedID: {CheckpointVersion: "refs-v1"},
            newID:         {Sessions: []checkpoint.SessionFilePaths{{Metadata: "new"}}},
        },
        metadata: map[id.CheckpointID][]checkpoint.CommittedMetadata{
            newID: {{
                SessionID: "new-session",
                CreatedAt: time.Date(2025, 1, 1, 11, 0, 0, 0, time.UTC),
            }},
        },
    }

latest, found, err := resolveLatestCheckpoint(context.Background(), reader, []id.CheckpointID{unsupportedID, newID})
    if err != nil {
        t.Fatalf("resolveLatestCheckpoint() error = %%v", err)
    }
    if !found {
        t.Fatal("resolveLatestCheckpoint() found = false")
    }
    if latest.CheckpointID != newID {
        t.Errorf("resolveLatestCheckpoint() = %s, want %s", latest.CheckpointID, newID)
    }
}

func TestResolveLatestCheckpointReturnsUnsupportedWhenNoReadableCheckpointExists(t *testing.T) {
    t.Parallel()

unsupportedID := id.MustCheckpointID("aaa111bbb222")
    reader := &resumeCheckpointInfoReaderStub{
        summaries: map[id.CheckpointID]*checkpoint.CheckpointSummary{
            unsupportedID: {CheckpointVersion: "refs-v1"},
        },
    }

_, found, err := resolveLatestCheckpoint(context.Background(), reader, []id.CheckpointID{unsupportedID})
    if err == nil {
        t.Fatal("resolveLatestCheckpoint() error = nil, want unsupported version")
    }
    if found {
        t.Fatal("resolveLatestCheckpoint() found = true")
    }
    if !checkpointpolicy.IsUnsupportedVersion(err) {
        t.Fatalf("resolveLatestCheckpoint() error = %%v, want unsupported version", err)
    }
}

type resumeCheckpointInfoReaderStub struct {
    summaries map[id.CheckpointID]*checkpoint.CheckpointSummary
    metadata  map[id.CheckpointID][]checkpoint.CommittedMetadata
}

Mcmd/entire/cli/resume_test.go+53

410 unmodified lines

411
412
413
414
415
416
417
418
419

410 unmodified lines

}

func UpdateCommandForCurrentBinary(currentVersion string) string {
    if !canAutoInstall() {
        return downloadsURL
    }
    return updateCommand(currentVersion)
}

Mcmd/entire/cli/versioncheck/versioncheck.go+3

345 unmodified lines

346
347
348
349
350
351
352
353
35 unmodified lines

389
390
391
390
392
393
394
395
30 unmodified lines

426
427
428
427
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
429
430
431
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475

345 unmodified lines

// it without tripping goconst on repeated string literals.
const brewUpgradeCmd = "brew upgrade entire"

const scoopExecutablePath = `C:\Users\test\scoop\apps\cli\current\entire.exe`

func TestUpdateCommand(t *testing.T) {
    const plainBinPath = "/usr/local/bin/entire"
    tests := []struct {
35 unmodified lines

{
            name:           "scoop path",
            currentVersion: "1.0.0",
            execPath:       func() (string, error) { return `C:\Users\test\scoop\apps\cli\current\entire.exe`, nil },
            execPath:       func() (string, error) { return scoopExecutablePath, nil },
            want:           "scoop update entire/cli",
        },
        {
30 unmodified lines

}

func TestUpdateCommandForCurrentBinary(t *testing.T) {
    t.Parallel()
    tests := []struct {
        name           string
        currentVersion string
        goos           string
        execPath       func() (string, error)
        want           string
    }{
        {
            name:           "known installer returns command",
            currentVersion: "1.2.3",
            goos:           goosWindows,
            execPath:       func() (string, error) { return scoopExecutablePath, nil },
            want:           "scoop update entire/cli",
        },
        {
            name:           "windows unknown installer returns releases URL",
            currentVersion: "1.2.3",
            goos:           goosWindows,
            execPath:       func() (string, error) { return `C:\Program Files\Entire\entire.exe`, nil },
            want:           downloadsURL,
        },
        {
            name:           "non-windows unknown installer returns curl command",
            currentVersion: "1.2.3",
            goos:           "linux",
            execPath:       func() (string, error) { return "/usr/local/bin/entire", nil },
            want:           "curl -fsSL https://entire.io/install.sh | bash",
        },
    }

got := UpdateCommandForCurrentBinary("1.2.3")
    if got == "" {
        t.Fatal("UpdateCommandForCurrentBinary returned empty command")
    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            originalExecPath := executablePath
            executablePath = tt.execPath
            t.Cleanup(func() { executablePath = originalExecPath })

originalGOOS := goos
            goos = tt.goos
            t.Cleanup(func() { goos = originalGOOS })

if got := UpdateCommandForCurrentBinary(tt.currentVersion); got != tt.want {
                t.Errorf("UpdateCommandForCurrentBinary() = %%q, want %%q", got, tt.want)
            }
        })
        }
    }
}