Address first review pass on Phase 2 checkpoint stores · Entire

Address first review pass on Phase 2 checkpoint stores

067d26f→main·

Soph·3w ago·5 files·+123 added/-28 removed

Fixes from the implementation review:

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

Sessions

ca5542401c18View transcript

Changes

5

9 unmodified lines

10
11
12
13
14
15
16
17
18
19
20
21
5 unmodified lines

27
28
29
30
31
32
33
62 unmodified lines

96
97
98
92
99
100
101
102
103
104
105
106
107
108
109
110
111
112
32 unmodified lines

145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
149 unmodified lines

310
311
312
313
314
315
316
317
318
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348

9 unmodified lines

// the git-specific blob-hash fields of the contract and stores transcript bytes
// directly, which keeps the example small and makes the contract's remaining
// git leakage concrete.
//
// It is faithful to the contract's per-session metadata and write-request
// semantics, but it does not replicate the git store's cross-session
// aggregation: the root summary's TokenUsage reflects the latest session rather
// than a sum across sessions. That aggregation is not needed to validate the
// pluggable seam and is intentionally omitted.
package fsstore

import (
5 unmodified lines

"sort"
    "strings"
    "sync"
    "time"

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

62 unmodified lines

if err != nil {
        return fmt.Errorf("fsstore: encode %s: %w", sc.Summary.CheckpointID, err)
    }
    if err := os.WriteFile(s.path(sc.Summary.CheckpointID), data, 0o644); err != nil { //nolint:gosec // reference test backend
        // 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 {
            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
}

32 unmodified lines

Prompts:    redact.String(strings.Join(opts.Prompts, checkpoint.PromptSeparator)),
    }
    sc.Sessions = upsertSession(sc.Sessions, session)

// Summary-level flags accumulate across sessions and survive recompute.
    sc.Summary.HasReview = sc.Summary.HasReview || opts.HasReview
    sc.Summary.HasInvestigation = sc.Summary.HasInvestigation || opts.HasInvestigation
    if opts.CombinedAttribution != nil {
        // Migration path: an initial write may carry holistic attribution. Normal
        // condensation sets this later via a CheckpointAttribution write instead.
        sc.Summary.CombinedAttribution = opts.CombinedAttribution
    }

recomputeSummary(sc)
    return s.save(sc)
}
149 unmodified lines

}

func metadataFromWriteOptions(opts cp.WriteOptions) cp.Metadata {
    createdAt := opts.CreatedAt
    if createdAt.IsZero() {
        // Contract: a zero CreatedAt means "use the current time".
        createdAt = time.Now()
    }
    return cp.Metadata{
        CheckpointID:     opts.CheckpointID,
        SessionID:        opts.SessionID,
        Strategy:         opts.Strategy,
        CreatedAt:        opts.CreatedAt,
        Branch:           opts.Branch,
        CheckpointsCount: opts.CheckpointsCount,
        SaveStepCount:    opts.SaveStepCount,
        FilesTouched:     opts.FilesTouched,
        Agent:            opts.Agent,
        Model:            opts.Model,
        TurnID:           opts.TurnID,
        IsTask:           opts.IsTask,
        ToolUseID:        opts.ToolUseID,
        TokenUsage:       opts.TokenUsage,
        SkillEvents:      opts.SkillEvents,
        SessionMetrics:   opts.SessionMetrics,
        Summary:          opts.Summary,
        Attribution:      opts.Attribution,
        Kind:             opts.Kind,
        ReviewSkills:     opts.ReviewSkills,
        ReviewPrompt:     opts.ReviewPrompt,
    }
}
// checkpoints config it resolves to the git backend with no mirrors, so default
// behavior is unchanged. When mirrors are configured, the persistent store is a
// fan-out wrapper (reads from primary, best-effort writes to each mirror).
//
// Backend selection is read via settings.LoadCheckpointsConfig, which resolves
// like settings.Load: from the context's worktree root if set, else relative to
// the current working directory — not from repo. Callers opening a repository
// that is not the cwd should wrap ctx with that worktree root (as dispatch does).
// Resolution is fail-soft: a missing or unreadable settings file yields the
// default git backend with no mirrors, preserving default behavior.
func Open(ctx context.Context, repo *git.Repository, opts OpenOptions) (*Stores, error) {
    refs := resolveOpenRefs(ctx, opts)
    env := OpenEnv{Repo: repo, BlobFetcher: opts.BlobFetcher, Refs: refs}
}

Mcmd/entire/cli/checkpoint/open.go+7

5 unmodified lines

6 7 8 9 10 11 12 13 14 15 16 39 unmodified lines

56 57 58 57 59 60 61 62 63 62 64 65 66 67 10 unmodified lines

78 79 80 79 81 82 83 82 83 84 85 86 87 88 89 90 91 85 92 93 94 95

5 unmodified lines

"encoding/json" "errors" "fmt" "log/slog" "os" "path/filepath"

"github.com/entireio/cli/cmd/entire/cli/logging" "github.com/entireio/cli/cmd/entire/cli/paths"

39 unmodified lines

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

cfg, err := loadCheckpointsFromFile(base) if err != nil { return nil, err } if local != "" { localCfg, err := loadCheckpointsFromFile(local) if err != nil { return nil, err } }

return cfg, nil }

func loadCheckpointsFromFile(filePath string) (*CheckpointsConfig, error) { data, err := os.ReadFile(filePath) //nolint:gosec // path is from AbsPath or a worktree-root join if err != nil { if os.IsNotExist(err) { return nil, nil //nolint:nilnil // absent file => no checkpoints config, not an error }
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, fmt.Errorf("reading settings file %s: %w", filePath, err) }

var env checkpointsEnvelope }

Mcmd/entire/cli/settings/checkpoints.go+13/-6


90 unmodified lines

91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106

90 unmodified lines

assert.Nil(t, cfg)
}

func TestLoadCheckpointsConfig_ToleratesUnreadableFile(t *testing.T) {
    dir := newCheckpointsSettingsRepo(t)
    // settings.json as a directory makes os.ReadFile fail with a non-ENOENT
    // error; the loader must stay fail-soft (no new failure for Open).
    require.NoError(t, os.MkdirAll(filepath.Join(dir, ".entire", "settings.json"), 0o755))
    cfg, err := LoadCheckpointsConfig(context.Background())
    require.NoError(t, err)
    assert.Nil(t, cfg)
}

func TestLoadCheckpointsConfig_LocalOverridesBase(t *testing.T) {
    dir := newCheckpointsSettingsRepo(t)
    writeFile(t, dir, "settings.json", `{"enabled": true, "checkpoints": {"primary": {"type": "git"}, "mirrors": [{"type": "fs"}]}}`)
}