Address PR bot review on Phase 2 checkpoint stores · Entire

Address PR bot review on Phase 2 checkpoint stores

1136bb3→main·Soph·3w ago·3 files·+51 added/-44 removed

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

Sessions

6ce520beb1ccView transcript

Changes

3

32 unmodified lines

33
34
35
36
37
38
39
57 unmodified lines

97
98
99
99
100
101
102
103
104
100
101
102
103
104
105
106
107
108
109
107
108
109

32 unmodified lines

cp "github.com/entireio/cli/api/checkpoint"
    "github.com/entireio/cli/cmd/entire/cli/checkpoint"
    "github.com/entireio/cli/cmd/entire/cli/checkpoint/id"
    "github.com/entireio/cli/cmd/entire/cli/jsonutil"

// BackendType is the registry type name for the filesystem reference backend.

57 unmodified lines

if err != nil {
        return fmt.Errorf("fsstore: encode %s: %w", sc.Summary.CheckpointID, err)
    }
    // Write atomically (temp + rename) so a reader never observes a partial
    // document. This guards a single writer's in-progress write; cross-process
    // concurrency is out of scope for this test-only backend.
    final := s.path(sc.Summary.CheckpointID)
    tmp := final + ".tmp"
    if err := os.WriteFile(tmp, data, 0o600); err != nil {
        // Write atomically via the shared helper (unique temp file, cleaned up on
        // failure) so a reader never observes a partial document. This guards a
        // single writer's in-progress write; cross-process concurrency is out of
        // scope for this test-only backend.
        if err := jsonutil.WriteFileAtomic(s.path(sc.Summary.CheckpointID), data, 0o600); err != nil {
            return fmt.Errorf("fsstore: write %s: %w", sc.Summary.CheckpointID, err)
        }
        if err := os.Rename(tmp, final); err != nil {
            return fmt.Errorf("fsstore: commit %s: %w", sc.Summary.CheckpointID, err)
        }
    }
    return nil
}

Mcmd/entire/cli/checkpoint/fsstore/fsstore.go+6/-9


53 unmodified lines

54
55
56
57
58
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
78
79
80
79
81
82
83
84
85
86
87
88
89
90
84
85
91
92
93
94
95
90
96
97
98
99
100
95
96
97
98
101
102
103
104
101
105
106
103
104
105
106
107
108
109
110
107
108
109
110

53 unmodified lines

func LoadCheckpointsConfig(ctx context.Context) (*CheckpointsConfig, error) {
    base, local := checkpointsSettingsPaths(ctx)

cfg, err := loadCheckpointsFromFile(ctx, base)
    if err != nil {
        // "local replaces base wholesale": prefer a checkpoints block from local
        // settings; fall back to base only when local has none. We extract the raw
        // blocks fail-soft, then decode/validate just the one that wins — so a
        // malformed block in the overridden file never blocks the file that wins.
        raw, src := rawCheckpointsBlock(ctx, local), local
        if raw == nil {
            raw, src = rawCheckpointsBlock(ctx, base), base
        }
        if raw == nil {
            return nil, nil //nolint:nilnil // no checkpoints block present => default git backend
        }

var cfg CheckpointsConfig
        dec := json.NewDecoder(bytes.NewReader(raw))
        dec.DisallowUnknownFields()
        if err := dec.Decode(&cfg); err != nil {
            return nil, fmt.Errorf("%w in %s: %w", ErrInvalidCheckpointsConfig, src, err)
        }
        if err := cfg.validate(); err != nil {
            return nil, err
        }
        if local != "" {
            localCfg, err := loadCheckpointsFromFile(ctx, local)
            if err != nil {
                return nil, err
            }
            if localCfg != nil {
                cfg = localCfg
            }
        }

if cfg != nil {
            if err := cfg.validate(); err != nil {
                return nil, err
            }
        }
        return cfg, nil
}

func loadCheckpointsFromFile(ctx context.Context, filePath string) (*CheckpointsConfig, error) {
    // rawCheckpointsBlock returns the raw "checkpoints" JSON block from filePath, or
    // nil when the file is absent/unreadable, has a whole-file syntax error, or has
    // no checkpoints block. It never errors: unrelated breakage in a settings file
    // must not block checkpoint construction (the strict Load path surfaces it for
    // normal commands).
    func rawCheckpointsBlock(ctx context.Context, filePath string) json.RawMessage {
        data, err := os.ReadFile(filePath) //nolint:gosec // path is from AbsPath or a worktree-root join
        if err != nil {
            if !os.IsNotExist(err) {
                // A non-ENOENT read error (bad perms, settings.json is a directory,
                // etc.) is a broken setup that the strict Load path surfaces for
                // normal commands. Stay fail-soft here so checkpoint construction
                // defaults to git rather than newly failing resume/explain/hooks.
                logging.Debug(ctx, "checkpoints config unreadable; defaulting to git backend",
                    slog.String("path", filePath), slog.String("error", err.Error()))
            }
            return nil, nil //nolint:nilnil // absent or unreadable file => no checkpoints config, fail-soft
        }

var env checkpointsEnvelope
        if err := json.Unmarshal(data, &env); err != nil {
            // A whole-file parse failure is an unrelated breakage that the strict
            // Load path surfaces for normal commands. Stay fail-soft here so
            // checkpoint construction defaults to git rather than failing.
            return nil, nil //nolint:nilnil,nilerr // intentionally fail-soft on unrelated malformed settings
        }
        if len(env.Checkpoints) == 0 {
            return nil, nil //nolint:nilnil // no checkpoints block present
        }

var cfg CheckpointsConfig
        dec := json.NewDecoder(bytes.NewReader(env.Checkpoints))
        dec.DisallowUnknownFields()
        if err := dec.Decode(&cfg); err != nil {
            return nil, fmt.Errorf("invalid checkpoints config in %s: %w", filePath, err)
        }
        return &cfg, nil
}

func (c *CheckpointsConfig) validate() error {