From 3e4757ffdcdb3d3261dcd76d0e19de1848c6e941 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Thu, 23 Apr 2026 18:09:32 +0200 Subject: [PATCH 1/9] guard attach checkpoints Entire-Checkpoint: e83cb707c70f --- cmd/entire/cli/attach.go | 50 +++++++++++++++ cmd/entire/cli/attach_test.go | 117 ++++++++++++++++++++++++++++++++++ 2 files changed, 167 insertions(+) diff --git a/cmd/entire/cli/attach.go b/cmd/entire/cli/attach.go index c2e3a5f713..717bd874b6 100644 --- a/cmd/entire/cli/attach.go +++ b/cmd/entire/cli/attach.go @@ -79,6 +79,7 @@ func runAttach(ctx context.Context, w io.Writer, sessionID string, agentName typ // Init failed — logging will use stderr fallback, non-fatal. _ = err } + defer logging.Close() logCtx := logging.WithComponent(ctx, "attach") @@ -135,8 +136,15 @@ func runAttach(ctx context.Context, w io.Writer, sessionID string, agentName typ // Determine checkpoint ID: reuse from HEAD if one exists, otherwise generate new. checkpointID, isExistingCheckpoint := resolveCheckpointID(headCommit) + // Refresh the metadata branch if HEAD already references a checkpoint. + // Returns a possibly-freshly-opened repo handle; see refreshForExistingCheckpoint. + repo = refreshForExistingCheckpoint(ctx, logCtx, repo, isExistingCheckpoint) + // Write directly to entire/checkpoints/v1. store := cpkg.NewGitStore(repo) + if err := verifyCheckpointLocallyAvailable(ctx, store, checkpointID, isExistingCheckpoint); err != nil { + return err + } author, err := GetGitAuthor(ctx) if err != nil { @@ -244,6 +252,48 @@ func getHeadCommit(repo *git.Repository) (*object.Commit, error) { return commit, nil } +// refreshForExistingCheckpoint refreshes the entire/checkpoints/v1 branch +// before reading or writing when HEAD already references a checkpoint. +// This mirrors `entire resume`'s metadata fetch fallback chain so attach +// doesn't operate on a stale local orphan branch after a normal `git pull`. +// Returns a freshly-opened repo handle so go-git sees any newly fetched +// packfiles; on fetch failure, logs a warning and returns the original repo. +func refreshForExistingCheckpoint(ctx, logCtx context.Context, repo *git.Repository, isExistingCheckpoint bool) *git.Repository { + if !isExistingCheckpoint { + return repo + } + _, freshRepo, fetchErr := getMetadataTree(ctx) + if fetchErr != nil { + logging.Warn(logCtx, "failed to refresh metadata branch before attach; proceeding with local state", + slog.String("error", fetchErr.Error())) + return repo + } + return freshRepo +} + +// verifyCheckpointLocallyAvailable refuses the attach when HEAD references +// an Entire-Checkpoint that is still missing from the local +// entire/checkpoints/v1 branch after refresh. Without this guard, attach +// would create a fresh checkpoint under the same ID and overwrite the +// original session data on push. +func verifyCheckpointLocallyAvailable(ctx context.Context, store *cpkg.GitStore, checkpointID id.CheckpointID, isExistingCheckpoint bool) error { + if !isExistingCheckpoint { + return nil + } + summary, readErr := store.ReadCommitted(ctx, checkpointID) + if readErr != nil { + return fmt.Errorf("failed to read checkpoint %s: %w", checkpointID, readErr) + } + if summary != nil { + return nil + } + const fetchCmd = "git fetch origin entire/checkpoints/v1:entire/checkpoints/v1" + return fmt.Errorf( + "checkpoint %s referenced by HEAD is missing from the local entire/checkpoints/v1 branch (the remote fetch didn't bring it in either). Creating a fresh checkpoint here would overwrite the original session data on push. Run:\n\n %s\n\nthen re-run attach. If the colleague who made this commit hasn't pushed their checkpoint metadata yet, ask them to do so first", + checkpointID.String(), fetchCmd, + ) +} + // resolveCheckpointID returns the checkpoint ID to use for the attach. // If HEAD already has an Entire-Checkpoint trailer, reuses that ID (the session // gets added as an additional session in the existing checkpoint). diff --git a/cmd/entire/cli/attach_test.go b/cmd/entire/cli/attach_test.go index 7eac6262b7..ae1a1f9d10 100644 --- a/cmd/entire/cli/attach_test.go +++ b/cmd/entire/cli/attach_test.go @@ -18,9 +18,11 @@ import ( _ "github.com/entireio/cli/cmd/entire/cli/agent/factoryaidroid" // register agent _ "github.com/entireio/cli/cmd/entire/cli/agent/geminicli" // register agent "github.com/entireio/cli/cmd/entire/cli/agent/types" + cpkg "github.com/entireio/cli/cmd/entire/cli/checkpoint" "github.com/entireio/cli/cmd/entire/cli/paths" "github.com/entireio/cli/cmd/entire/cli/session" "github.com/entireio/cli/cmd/entire/cli/testutil" + "github.com/entireio/cli/cmd/entire/cli/trailers" "github.com/go-git/go-git/v6" "github.com/go-git/go-git/v6/plumbing" @@ -255,6 +257,121 @@ func TestAttach_V2DualWriteDisabled(t *testing.T) { } } +func TestAttach_AppendsAsAdditionalSessionWhenIDDiffers(t *testing.T) { + setupAttachTestRepo(t) + + firstSessionID := "first-session-a-original" + setupClaudeTranscript(t, firstSessionID, `{"type":"user","message":{"role":"user","content":"first"},"uuid":"u1"} +`) + var out bytes.Buffer + if err := runAttach(context.Background(), &out, firstSessionID, agent.AgentNameClaudeCode, true); err != nil { + t.Fatalf("first attach failed: %v", err) + } + + repoRoot := mustGetwd(t) + repo, err := git.PlainOpen(repoRoot) + if err != nil { + t.Fatal(err) + } + headRef, err := repo.Head() + if err != nil { + t.Fatal(err) + } + headCommit, err := repo.CommitObject(headRef.Hash()) + if err != nil { + t.Fatal(err) + } + existingCheckpoints := trailers.ParseAllCheckpoints(headCommit.Message) + if len(existingCheckpoints) != 1 { + t.Fatalf("expected one Entire-Checkpoint trailer after first attach; got %v", existingCheckpoints) + } + checkpointID := existingCheckpoints[0] + + secondSessionID := "second-session-b-append" + setupClaudeTranscript(t, secondSessionID, `{"type":"user","message":{"role":"user","content":"second"},"uuid":"u1"} +`) + out.Reset() + if err := runAttach(context.Background(), &out, secondSessionID, agent.AgentNameClaudeCode, true); err != nil { + t.Fatalf("second attach failed: %v", err) + } + + store := cpkg.NewGitStore(repo) + summary, err := store.ReadCommitted(context.Background(), checkpointID) + if err != nil { + t.Fatalf("ReadCommitted(%s): %v", checkpointID, err) + } + if summary == nil { + t.Fatalf("checkpoint %s summary nil after two attaches", checkpointID) + } + if len(summary.Sessions) != 2 { + t.Fatalf("checkpoint has %d sessions, want 2", len(summary.Sessions)) + } + + idx0, err := store.ReadSessionContent(context.Background(), checkpointID, 0) + if err != nil { + t.Fatalf("ReadSessionContent(0): %v", err) + } + idx1, err := store.ReadSessionContent(context.Background(), checkpointID, 1) + if err != nil { + t.Fatalf("ReadSessionContent(1): %v", err) + } + haveFirst := idx0.Metadata.SessionID == firstSessionID || idx1.Metadata.SessionID == firstSessionID + haveSecond := idx0.Metadata.SessionID == secondSessionID || idx1.Metadata.SessionID == secondSessionID + if !haveFirst { + t.Errorf("first session %q missing from checkpoint; got [%q, %q]", + firstSessionID, idx0.Metadata.SessionID, idx1.Metadata.SessionID) + } + if !haveSecond { + t.Errorf("second session %q missing from checkpoint; got [%q, %q]", + secondSessionID, idx0.Metadata.SessionID, idx1.Metadata.SessionID) + } +} + +func TestAttach_RefusesWhenCheckpointMissingFromLocalBranch(t *testing.T) { + setupAttachTestRepo(t) + + repoRoot := mustGetwd(t) + runGitInDir(t, repoRoot, "commit", "--amend", "--no-edit", "-m", "init\n\nEntire-Checkpoint: ffffffffeeee") + + sessionID := "orphaned-attach-session" + setupClaudeTranscript(t, sessionID, `{"type":"user","message":{"role":"user","content":"attach please"},"uuid":"u1"} +`) + + var out bytes.Buffer + err := runAttach(context.Background(), &out, sessionID, agent.AgentNameClaudeCode, true) + if err == nil { + t.Fatal("expected error: checkpoint referenced by HEAD is missing locally and attach should refuse") + } + if !strings.Contains(err.Error(), "missing from the local entire/checkpoints/v1 branch") { + t.Errorf("error message should explain the missing-branch situation; got: %v", err) + } + if !strings.Contains(err.Error(), "git fetch origin entire/checkpoints/v1") { + t.Errorf("error message should include the fetch command to fix it; got: %v", err) + } + + repo, err := git.PlainOpen(repoRoot) + if err != nil { + t.Fatal(err) + } + store := cpkg.NewGitStore(repo) + summary, err := store.ReadCommitted(context.Background(), "ffffffffeeee") + if err != nil { + t.Fatalf("ReadCommitted: %v", err) + } + if summary != nil { + t.Errorf("attach should NOT have created checkpoint ffffffffeeee locally; found %+v", summary) + } +} + +func runGitInDir(t *testing.T, dir string, args ...string) { + t.Helper() + cmd := exec.CommandContext(context.Background(), "git", args...) + cmd.Dir = dir + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v in %s: %v\n%s", args, dir, err, out) + } +} + func TestAttach_PopulatesTokenUsage(t *testing.T) { setupAttachTestRepo(t) From 5a875d5887fc3f141f79b7563014e050596f14e4 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Thu, 23 Apr 2026 18:15:09 +0200 Subject: [PATCH 2/9] warn on checkpoint overwrite Entire-Checkpoint: 321e85e6b869 --- cmd/entire/cli/checkpoint/committed.go | 23 +++++ .../cli/checkpoint/committed_tripwire_test.go | 91 +++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 cmd/entire/cli/checkpoint/committed_tripwire_test.go diff --git a/cmd/entire/cli/checkpoint/committed.go b/cmd/entire/cli/checkpoint/committed.go index 16f9dfec26..6bc23de60c 100644 --- a/cmd/entire/cli/checkpoint/committed.go +++ b/cmd/entire/cli/checkpoint/committed.go @@ -317,6 +317,18 @@ func (s *GitStore) writeStandardCheckpointEntries(ctx context.Context, opts Writ // Determine session index: reuse existing slot if session ID matches, otherwise append sessionIndex := s.findSessionIndex(ctx, basePath, existingSummary, entries, opts.SessionID) + // Capture any pre-existing session-0 metadata before writeSessionToSubdirectory + // clears that subtree. This is a diagnostic tripwire for a production report + // where session 0 was silently replaced by a different session's data. + var existingSessionZeroMeta *CommittedMetadata + if sessionIndex == 0 { + if entry, exists := entries[fmt.Sprintf("%s0/%s", basePath, paths.MetadataFileName)]; exists { + if existingMeta, readErr := s.readMetadataFromBlob(entry.Hash); readErr == nil { + existingSessionZeroMeta = existingMeta + } + } + } + // Write session files to numbered subdirectory sessionPath := fmt.Sprintf("%s%d/", basePath, sessionIndex) sessionFilePaths, err := s.writeSessionToSubdirectory(ctx, opts, sessionPath, entries) @@ -341,6 +353,17 @@ func (s *GitStore) writeStandardCheckpointEntries(ctx context.Context, opts Writ } sessions[sessionIndex] = sessionFilePaths + // Tripwire: if we're writing session 0 and there was already session-0 + // metadata for a DIFFERENT session ID, emit a loud warning so we have a + // log trace instead of only the overwrite symptom. + if existingSessionZeroMeta != nil && existingSessionZeroMeta.SessionID != opts.SessionID { + logging.Warn(ctx, "checkpoint write overwrites session 0 with a different sessionID — potential overwrite regression", + slog.String("checkpoint_id", opts.CheckpointID.String()), + slog.String("existing_session_id", existingSessionZeroMeta.SessionID), + slog.String("write_session_id", opts.SessionID), + slog.Bool("existing_summary_nil", existingSummary == nil)) + } + // Update root metadata.json with CheckpointSummary return s.writeCheckpointSummary(opts, basePath, entries, sessions) } diff --git a/cmd/entire/cli/checkpoint/committed_tripwire_test.go b/cmd/entire/cli/checkpoint/committed_tripwire_test.go new file mode 100644 index 0000000000..825fc1709f --- /dev/null +++ b/cmd/entire/cli/checkpoint/committed_tripwire_test.go @@ -0,0 +1,91 @@ +package checkpoint + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/entireio/cli/cmd/entire/cli/checkpoint/id" + "github.com/entireio/cli/cmd/entire/cli/jsonutil" + "github.com/entireio/cli/cmd/entire/cli/logging" + "github.com/entireio/cli/cmd/entire/cli/paths" + "github.com/entireio/cli/cmd/entire/cli/versioninfo" + "github.com/entireio/cli/redact" + + "github.com/go-git/go-git/v6" + "github.com/go-git/go-git/v6/plumbing/filemode" + "github.com/go-git/go-git/v6/plumbing/object" +) + +func TestWriteStandardCheckpointEntries_WarnsOnUnexpectedSessionZeroOverwrite(t *testing.T) { + tmpDir := t.TempDir() + t.Chdir(tmpDir) + + repo, err := git.PlainInit(tmpDir, false) + if err != nil { + t.Fatalf("PlainInit() error = %v", err) + } + store := NewGitStore(repo) + + if err := logging.Init(context.Background(), ""); err != nil { + t.Fatalf("logging.Init() error = %v", err) + } + defer logging.Close() + + checkpointID, err := id.Generate() + if err != nil { + t.Fatalf("id.Generate() error = %v", err) + } + basePath := checkpointID.Path() + "/" + + oldMetadata := CommittedMetadata{ + CheckpointID: checkpointID, + SessionID: "session-old", + Strategy: "manual-commit", + CLIVersion: versioninfo.Version, + } + oldMetadataJSON, err := jsonutil.MarshalIndentWithNewline(oldMetadata, "", " ") + if err != nil { + t.Fatalf("marshal old metadata: %v", err) + } + oldMetadataHash, err := CreateBlobFromContent(repo, oldMetadataJSON) + if err != nil { + t.Fatalf("CreateBlobFromContent(old metadata) error = %v", err) + } + + entries := map[string]object.TreeEntry{ + basePath + "0/" + paths.MetadataFileName: { + Name: basePath + "0/" + paths.MetadataFileName, + Mode: filemode.Regular, + Hash: oldMetadataHash, + }, + } + + opts := WriteCommittedOptions{ + CheckpointID: checkpointID, + SessionID: "session-new", + Strategy: "manual-commit", + Transcript: redact.AlreadyRedacted([]byte("{\"type\":\"user\",\"message\":\"hi\"}\n")), + Prompts: []string{"hi"}, + } + + if err := store.writeStandardCheckpointEntries(context.Background(), opts, basePath, entries); err != nil { + t.Fatalf("writeStandardCheckpointEntries() error = %v", err) + } + logging.Close() + + logPath := filepath.Join(tmpDir, logging.LogsDir, "entire.log") + logData, err := os.ReadFile(logPath) + if err != nil { + t.Fatalf("ReadFile(%s) error = %v", logPath, err) + } + logText := string(logData) + if !strings.Contains(logText, "checkpoint write overwrites session 0 with a different sessionID") { + t.Fatalf("expected tripwire warning in log, got:\n%s", logText) + } + if !strings.Contains(logText, "session-old") || !strings.Contains(logText, "session-new") { + t.Fatalf("expected log to include both session IDs, got:\n%s", logText) + } +} From 6fd6d491837bb4b60cf07e2083b1e298a0ce4fbf Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Thu, 23 Apr 2026 18:38:42 +0200 Subject: [PATCH 3/9] polish attach guards Entire-Checkpoint: e7316f61963c --- cmd/entire/cli/attach.go | 2 +- cmd/entire/cli/attach_test.go | 20 +++++++++---------- cmd/entire/cli/checkpoint/committed.go | 10 ++++++---- .../cli/checkpoint/committed_tripwire_test.go | 1 - 4 files changed, 17 insertions(+), 16 deletions(-) diff --git a/cmd/entire/cli/attach.go b/cmd/entire/cli/attach.go index 717bd874b6..e9c87c3fa0 100644 --- a/cmd/entire/cli/attach.go +++ b/cmd/entire/cli/attach.go @@ -289,7 +289,7 @@ func verifyCheckpointLocallyAvailable(ctx context.Context, store *cpkg.GitStore, } const fetchCmd = "git fetch origin entire/checkpoints/v1:entire/checkpoints/v1" return fmt.Errorf( - "checkpoint %s referenced by HEAD is missing from the local entire/checkpoints/v1 branch (the remote fetch didn't bring it in either). Creating a fresh checkpoint here would overwrite the original session data on push. Run:\n\n %s\n\nthen re-run attach. If the colleague who made this commit hasn't pushed their checkpoint metadata yet, ask them to do so first", + "checkpoint %s referenced by HEAD is missing from the local entire/checkpoints/v1 branch after a refresh attempt. Creating a fresh checkpoint here would overwrite the original session data on push. Run:\n\n %s\n\nthen re-run attach. If the colleague who made this commit hasn't pushed their checkpoint metadata yet, ask them to do so first", checkpointID.String(), fetchCmd, ) } diff --git a/cmd/entire/cli/attach_test.go b/cmd/entire/cli/attach_test.go index ae1a1f9d10..4a1dba9e64 100644 --- a/cmd/entire/cli/attach_test.go +++ b/cmd/entire/cli/attach_test.go @@ -331,7 +331,7 @@ func TestAttach_RefusesWhenCheckpointMissingFromLocalBranch(t *testing.T) { setupAttachTestRepo(t) repoRoot := mustGetwd(t) - runGitInDir(t, repoRoot, "commit", "--amend", "--no-edit", "-m", "init\n\nEntire-Checkpoint: ffffffffeeee") + runGitInDir(t, repoRoot, "commit", "--amend", "-m", "init\n\nEntire-Checkpoint: ffffffffeeee") sessionID := "orphaned-attach-session" setupClaudeTranscript(t, sessionID, `{"type":"user","message":{"role":"user","content":"attach please"},"uuid":"u1"} @@ -363,15 +363,6 @@ func TestAttach_RefusesWhenCheckpointMissingFromLocalBranch(t *testing.T) { } } -func runGitInDir(t *testing.T, dir string, args ...string) { - t.Helper() - cmd := exec.CommandContext(context.Background(), "git", args...) - cmd.Dir = dir - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatalf("git %v in %s: %v\n%s", args, dir, err, out) - } -} - func TestAttach_PopulatesTokenUsage(t *testing.T) { setupAttachTestRepo(t) @@ -964,3 +955,12 @@ func TestAttach_DiscoversExternalAgents(t *testing.T) { t.Errorf("expected external agent %q in registry after attach, got: %v", agentName, lookupErr) } } + +func runGitInDir(t *testing.T, dir string, args ...string) { + t.Helper() + cmd := exec.CommandContext(context.Background(), "git", args...) + cmd.Dir = dir + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %v in %s: %v\n%s", args, dir, err, out) + } +} diff --git a/cmd/entire/cli/checkpoint/committed.go b/cmd/entire/cli/checkpoint/committed.go index 6bc23de60c..4e36ffbdfe 100644 --- a/cmd/entire/cli/checkpoint/committed.go +++ b/cmd/entire/cli/checkpoint/committed.go @@ -318,8 +318,10 @@ func (s *GitStore) writeStandardCheckpointEntries(ctx context.Context, opts Writ sessionIndex := s.findSessionIndex(ctx, basePath, existingSummary, entries, opts.SessionID) // Capture any pre-existing session-0 metadata before writeSessionToSubdirectory - // clears that subtree. This is a diagnostic tripwire for a production report - // where session 0 was silently replaced by a different session's data. + // clears that subtree. The warning below only fires in the suspicious shape + // where findSessionIndex chose slot 0 but the tree already had session-0 + // metadata for a different session — typically meaning the root summary is + // missing/stale while a numbered session subdir still exists. var existingSessionZeroMeta *CommittedMetadata if sessionIndex == 0 { if entry, exists := entries[fmt.Sprintf("%s0/%s", basePath, paths.MetadataFileName)]; exists { @@ -354,8 +356,8 @@ func (s *GitStore) writeStandardCheckpointEntries(ctx context.Context, opts Writ sessions[sessionIndex] = sessionFilePaths // Tripwire: if we're writing session 0 and there was already session-0 - // metadata for a DIFFERENT session ID, emit a loud warning so we have a - // log trace instead of only the overwrite symptom. + // metadata for a DIFFERENT session ID, emit a loud warning. This is a + // tree-corruption / stale-summary shape, not a routine overwrite path. if existingSessionZeroMeta != nil && existingSessionZeroMeta.SessionID != opts.SessionID { logging.Warn(ctx, "checkpoint write overwrites session 0 with a different sessionID — potential overwrite regression", slog.String("checkpoint_id", opts.CheckpointID.String()), diff --git a/cmd/entire/cli/checkpoint/committed_tripwire_test.go b/cmd/entire/cli/checkpoint/committed_tripwire_test.go index 825fc1709f..be82836f66 100644 --- a/cmd/entire/cli/checkpoint/committed_tripwire_test.go +++ b/cmd/entire/cli/checkpoint/committed_tripwire_test.go @@ -32,7 +32,6 @@ func TestWriteStandardCheckpointEntries_WarnsOnUnexpectedSessionZeroOverwrite(t if err := logging.Init(context.Background(), ""); err != nil { t.Fatalf("logging.Init() error = %v", err) } - defer logging.Close() checkpointID, err := id.Generate() if err != nil { From a5be08cc568afd5f532592a08de866adb20e1de5 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Thu, 23 Apr 2026 19:39:38 +0200 Subject: [PATCH 4/9] fast-path local checkpoint, smarter fetch hint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skip the metadata refresh when the checkpoint is already on the local entire/checkpoints/v1 branch — no network round-trip on repeat attaches. When the refuse path fires, suggest a fetch from the configured checkpoint_remote URL if one is set, instead of unconditionally pointing users at origin. Entire-Checkpoint: 941a2b7d55bf --- cmd/entire/cli/attach.go | 90 ++++++++++++++++++++++++---------------- 1 file changed, 54 insertions(+), 36 deletions(-) diff --git a/cmd/entire/cli/attach.go b/cmd/entire/cli/attach.go index e9c87c3fa0..d46ebbfeab 100644 --- a/cmd/entire/cli/attach.go +++ b/cmd/entire/cli/attach.go @@ -136,15 +136,16 @@ func runAttach(ctx context.Context, w io.Writer, sessionID string, agentName typ // Determine checkpoint ID: reuse from HEAD if one exists, otherwise generate new. checkpointID, isExistingCheckpoint := resolveCheckpointID(headCommit) - // Refresh the metadata branch if HEAD already references a checkpoint. - // Returns a possibly-freshly-opened repo handle; see refreshForExistingCheckpoint. - repo = refreshForExistingCheckpoint(ctx, logCtx, repo, isExistingCheckpoint) + // If HEAD references an existing checkpoint, make sure we have it locally + // before writing — otherwise we'd create a fresh session 0 under the same + // ID and overwrite the original on push. + repo, err = ensureCheckpointAvailable(ctx, logCtx, repo, checkpointID, isExistingCheckpoint) + if err != nil { + return err + } // Write directly to entire/checkpoints/v1. store := cpkg.NewGitStore(repo) - if err := verifyCheckpointLocallyAvailable(ctx, store, checkpointID, isExistingCheckpoint); err != nil { - return err - } author, err := GetGitAuthor(ctx) if err != nil { @@ -252,48 +253,65 @@ func getHeadCommit(repo *git.Repository) (*object.Commit, error) { return commit, nil } -// refreshForExistingCheckpoint refreshes the entire/checkpoints/v1 branch -// before reading or writing when HEAD already references a checkpoint. -// This mirrors `entire resume`'s metadata fetch fallback chain so attach -// doesn't operate on a stale local orphan branch after a normal `git pull`. -// Returns a freshly-opened repo handle so go-git sees any newly fetched -// packfiles; on fetch failure, logs a warning and returns the original repo. -func refreshForExistingCheckpoint(ctx, logCtx context.Context, repo *git.Repository, isExistingCheckpoint bool) *git.Repository { +// ensureCheckpointAvailable makes sure the checkpoint referenced by HEAD is +// present locally before the attach writes to it. Without this guard, attach +// would create a fresh session 0 under the same ID and overwrite the original +// session data on push. +// +// Checks the local entire/checkpoints/v1 branch first — if the checkpoint is +// already there, no network is needed. Otherwise triggers the metadata fetch +// fallback chain used by `entire resume` and re-checks. Returns a +// possibly-freshly-opened repo handle so go-git sees any newly fetched +// packfiles. +func ensureCheckpointAvailable(ctx, logCtx context.Context, repo *git.Repository, checkpointID id.CheckpointID, isExistingCheckpoint bool) (*git.Repository, error) { if !isExistingCheckpoint { - return repo + return repo, nil } - _, freshRepo, fetchErr := getMetadataTree(ctx) - if fetchErr != nil { - logging.Warn(logCtx, "failed to refresh metadata branch before attach; proceeding with local state", - slog.String("error", fetchErr.Error())) - return repo - } - return freshRepo -} -// verifyCheckpointLocallyAvailable refuses the attach when HEAD references -// an Entire-Checkpoint that is still missing from the local -// entire/checkpoints/v1 branch after refresh. Without this guard, attach -// would create a fresh checkpoint under the same ID and overwrite the -// original session data on push. -func verifyCheckpointLocallyAvailable(ctx context.Context, store *cpkg.GitStore, checkpointID id.CheckpointID, isExistingCheckpoint bool) error { - if !isExistingCheckpoint { - return nil - } + // Fast path: already local, skip the network round-trip. + store := cpkg.NewGitStore(repo) summary, readErr := store.ReadCommitted(ctx, checkpointID) if readErr != nil { - return fmt.Errorf("failed to read checkpoint %s: %w", checkpointID, readErr) + return repo, fmt.Errorf("failed to read checkpoint %s: %w", checkpointID, readErr) } if summary != nil { - return nil + return repo, nil + } + + // Missing locally — try to refresh, then re-check. + if _, freshRepo, fetchErr := getMetadataTree(ctx); fetchErr != nil { + logging.Warn(logCtx, "failed to refresh metadata branch before attach; proceeding with local state", + slog.String("error", fetchErr.Error())) + } else { + repo = freshRepo + store = cpkg.NewGitStore(repo) + summary, readErr = store.ReadCommitted(ctx, checkpointID) + if readErr != nil { + return repo, fmt.Errorf("failed to read checkpoint %s after refresh: %w", checkpointID, readErr) + } + if summary != nil { + return repo, nil + } } - const fetchCmd = "git fetch origin entire/checkpoints/v1:entire/checkpoints/v1" - return fmt.Errorf( + + return repo, fmt.Errorf( "checkpoint %s referenced by HEAD is missing from the local entire/checkpoints/v1 branch after a refresh attempt. Creating a fresh checkpoint here would overwrite the original session data on push. Run:\n\n %s\n\nthen re-run attach. If the colleague who made this commit hasn't pushed their checkpoint metadata yet, ask them to do so first", - checkpointID.String(), fetchCmd, + checkpointID.String(), suggestCheckpointFetchCommand(logCtx), ) } +// suggestCheckpointFetchCommand returns a git fetch command string the user +// can paste, aware of whether checkpoint_remote is configured. +func suggestCheckpointFetchCommand(ctx context.Context) string { + const ref = "entire/checkpoints/v1:entire/checkpoints/v1" + if remote.Configured(ctx) { + if url, err := remote.FetchURL(ctx); err == nil && url != "" { + return fmt.Sprintf("git fetch %s %s", url, ref) + } + } + return "git fetch origin " + ref +} + // resolveCheckpointID returns the checkpoint ID to use for the attach. // If HEAD already has an Entire-Checkpoint trailer, reuses that ID (the session // gets added as an additional session in the existing checkpoint). From cf2dbdd961c42df2661703eb3534fe0ee4fa9ae2 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Thu, 23 Apr 2026 20:25:27 +0200 Subject: [PATCH 5/9] plug remote-tracking loophole in attach guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fast path was reading the checkpoint via the go-git store, which silently falls back to refs/remotes/origin/entire/checkpoints/v1 when the local branch is missing. So a freshly-cloned repo where the checkpoint exists only on the remote-tracking ref passed the guard, then WriteCommitted created an orphan local branch with an empty tree, and the push would have clobbered the remote — the exact case the guard is meant to block. Check for the local branch ref explicitly before trusting the read. In v2-only mode, use the v2 store instead (refs/entire/main has no remote-tracking analog, so the equivalent of this hole doesn't exist there). Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: eb718cd31155 --- cmd/entire/cli/attach.go | 72 +++++++++++++++++++++++++------ cmd/entire/cli/attach_test.go | 80 +++++++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 14 deletions(-) diff --git a/cmd/entire/cli/attach.go b/cmd/entire/cli/attach.go index d46ebbfeab..413d4d4b21 100644 --- a/cmd/entire/cli/attach.go +++ b/cmd/entire/cli/attach.go @@ -20,6 +20,7 @@ import ( "github.com/entireio/cli/cmd/entire/cli/checkpoint/remote" "github.com/entireio/cli/cmd/entire/cli/interactive" "github.com/entireio/cli/cmd/entire/cli/logging" + "github.com/entireio/cli/cmd/entire/cli/paths" "github.com/entireio/cli/cmd/entire/cli/session" "github.com/entireio/cli/cmd/entire/cli/settings" "github.com/entireio/cli/cmd/entire/cli/strategy" @@ -31,6 +32,7 @@ import ( "github.com/charmbracelet/huh" "github.com/go-git/go-git/v6" + "github.com/go-git/go-git/v6/plumbing" "github.com/go-git/go-git/v6/plumbing/object" "github.com/spf13/cobra" ) @@ -258,23 +260,27 @@ func getHeadCommit(repo *git.Repository) (*object.Commit, error) { // would create a fresh session 0 under the same ID and overwrite the original // session data on push. // -// Checks the local entire/checkpoints/v1 branch first — if the checkpoint is -// already there, no network is needed. Otherwise triggers the metadata fetch -// fallback chain used by `entire resume` and re-checks. Returns a -// possibly-freshly-opened repo handle so go-git sees any newly fetched -// packfiles. +// Only the local branch counts — remote-tracking presence is not enough. +// If only the remote-tracking ref exists, a subsequent WriteCommitted creates +// a brand-new orphan local branch with an empty tree, which would clobber +// the remote on push. +// +// Fast path: check local refs directly — no network. If missing, trigger the +// metadata fetch fallback chain used by `entire resume` (which advances the +// local ref on success) and re-check. Returns a possibly-freshly-opened repo +// handle so go-git sees any newly fetched packfiles. func ensureCheckpointAvailable(ctx, logCtx context.Context, repo *git.Repository, checkpointID id.CheckpointID, isExistingCheckpoint bool) (*git.Repository, error) { if !isExistingCheckpoint { return repo, nil } - // Fast path: already local, skip the network round-trip. - store := cpkg.NewGitStore(repo) - summary, readErr := store.ReadCommitted(ctx, checkpointID) + v2Only := settings.CheckpointsVersion(logCtx) == 2 + + present, readErr := checkpointPresentLocally(ctx, repo, checkpointID, v2Only) if readErr != nil { return repo, fmt.Errorf("failed to read checkpoint %s: %w", checkpointID, readErr) } - if summary != nil { + if present { return repo, nil } @@ -284,22 +290,60 @@ func ensureCheckpointAvailable(ctx, logCtx context.Context, repo *git.Repository slog.String("error", fetchErr.Error())) } else { repo = freshRepo - store = cpkg.NewGitStore(repo) - summary, readErr = store.ReadCommitted(ctx, checkpointID) + present, readErr = checkpointPresentLocally(ctx, repo, checkpointID, v2Only) if readErr != nil { return repo, fmt.Errorf("failed to read checkpoint %s after refresh: %w", checkpointID, readErr) } - if summary != nil { + if present { return repo, nil } } + branchDescription := "entire/checkpoints/v1 branch" + if v2Only { + branchDescription = "v2 /main ref" + } return repo, fmt.Errorf( - "checkpoint %s referenced by HEAD is missing from the local entire/checkpoints/v1 branch after a refresh attempt. Creating a fresh checkpoint here would overwrite the original session data on push. Run:\n\n %s\n\nthen re-run attach. If the colleague who made this commit hasn't pushed their checkpoint metadata yet, ask them to do so first", - checkpointID.String(), suggestCheckpointFetchCommand(logCtx), + "checkpoint %s referenced by HEAD is missing from the local %s after a refresh attempt. Creating a fresh checkpoint here would overwrite the original session data on push. Run:\n\n %s\n\nthen re-run attach. If the colleague who made this commit hasn't pushed their checkpoint metadata yet, ask them to do so first", + checkpointID.String(), branchDescription, suggestCheckpointFetchCommand(logCtx), ) } +// checkpointPresentLocally reports whether the checkpoint already exists on +// the local ref we would write to. For v1 / dual-write, that's the local +// entire/checkpoints/v1 branch (remote-tracking alone is not enough — see +// ensureCheckpointAvailable). For v2-only mode, it's the v2 /main ref, which +// has no remote-tracking analog and is therefore already local-only by +// construction. +func checkpointPresentLocally(ctx context.Context, repo *git.Repository, checkpointID id.CheckpointID, v2Only bool) (bool, error) { + if v2Only { + v2URL, urlErr := remote.FetchURL(ctx) + if urlErr != nil { + logging.Debug(ctx, "attach: using origin for v2 store fetch remote", + slog.String("error", urlErr.Error()), + ) + } + summary, err := cpkg.NewV2GitStore(repo, v2URL).ReadCommitted(ctx, checkpointID) + if err != nil { + return false, err //nolint:wrapcheck // Caller wraps with checkpoint ID context + } + return summary != nil, nil + } + + localRef := plumbing.NewBranchReferenceName(paths.MetadataBranchName) + if _, err := repo.Reference(localRef, true); err != nil { + // Local branch ref doesn't exist — treat as "not present locally". + // We deliberately do not fall back to remote-tracking: see + // ensureCheckpointAvailable's docstring. + return false, nil //nolint:nilerr // Missing ref is the "absent" signal, not an error. + } + summary, err := cpkg.NewGitStore(repo).ReadCommitted(ctx, checkpointID) + if err != nil { + return false, err //nolint:wrapcheck // Caller wraps with checkpoint ID context + } + return summary != nil, nil +} + // suggestCheckpointFetchCommand returns a git fetch command string the user // can paste, aware of whether checkpoint_remote is configured. func suggestCheckpointFetchCommand(ctx context.Context) string { diff --git a/cmd/entire/cli/attach_test.go b/cmd/entire/cli/attach_test.go index 4a1dba9e64..8a5f7a16ab 100644 --- a/cmd/entire/cli/attach_test.go +++ b/cmd/entire/cli/attach_test.go @@ -19,10 +19,12 @@ import ( _ "github.com/entireio/cli/cmd/entire/cli/agent/geminicli" // register agent "github.com/entireio/cli/cmd/entire/cli/agent/types" cpkg "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/session" "github.com/entireio/cli/cmd/entire/cli/testutil" "github.com/entireio/cli/cmd/entire/cli/trailers" + "github.com/entireio/cli/redact" "github.com/go-git/go-git/v6" "github.com/go-git/go-git/v6/plumbing" @@ -363,6 +365,84 @@ func TestAttach_RefusesWhenCheckpointMissingFromLocalBranch(t *testing.T) { } } +// Regression for https://github.com/entireio/cli/pull/1014#pullrequestreview-copilot: +// Bob clones a repo where Alice's checkpoint is on the remote-tracking ref +// (refs/remotes/origin/entire/checkpoints/v1) but the local branch doesn't +// exist yet. ReadCommitted falls back to the remote-tracking tree, so a naive +// "read and check" guard would think all is well. But WriteCommitted would +// then create a *fresh* orphan local branch, and Bob's push would clobber +// Alice's data on origin. Attach must refuse in this shape. +func TestAttach_RefusesWhenCheckpointOnlyInRemoteTrackingRef(t *testing.T) { + setupAttachTestRepo(t) + + repoRoot := mustGetwd(t) + repo, err := git.PlainOpen(repoRoot) + if err != nil { + t.Fatal(err) + } + + // Seed the local branch with a checkpoint representing Alice's session. + alicesCheckpoint := id.MustCheckpointID("abcdef012345") + store := cpkg.NewGitStore(repo) + if writeErr := store.WriteCommitted(context.Background(), cpkg.WriteCommittedOptions{ + CheckpointID: alicesCheckpoint, + SessionID: "alice-original", + Strategy: "manual-commit", + Transcript: redact.AlreadyRedacted([]byte(`{"type":"user","message":"hi"}` + "\n")), + AuthorName: "Alice", + AuthorEmail: "alice@example.com", + }); writeErr != nil { + t.Fatalf("WriteCommitted: %v", writeErr) + } + + // Move the populated branch to the remote-tracking ref, then delete the + // local ref. This is the shape `git clone` produces for a branch the user + // never explicitly checked out locally. + localRef := plumbing.NewBranchReferenceName(paths.MetadataBranchName) + remoteTrackingRef := plumbing.NewRemoteReferenceName("origin", paths.MetadataBranchName) + populated, err := repo.Reference(localRef, true) + if err != nil { + t.Fatalf("reading local ref: %v", err) + } + if setErr := repo.Storer.SetReference(plumbing.NewHashReference(remoteTrackingRef, populated.Hash())); setErr != nil { + t.Fatalf("SetReference remote-tracking: %v", setErr) + } + if rmErr := repo.Storer.RemoveReference(localRef); rmErr != nil { + t.Fatalf("RemoveReference local: %v", rmErr) + } + + // Amend HEAD so attach treats this as an existing-checkpoint case. + runGitInDir(t, repoRoot, "commit", "--amend", "-m", "init\n\nEntire-Checkpoint: "+alicesCheckpoint.String()) + + sessionID := "bob-attempted-attach" + setupClaudeTranscript(t, sessionID, `{"type":"user","message":{"role":"user","content":"hi"},"uuid":"u1"} +`) + + var out bytes.Buffer + err = runAttach(context.Background(), &out, sessionID, agent.AgentNameClaudeCode, true) + if err == nil { + t.Fatal("expected attach to refuse when checkpoint is only in the remote-tracking ref") + } + if !strings.Contains(err.Error(), "missing from the local entire/checkpoints/v1 branch") { + t.Errorf("error should explain the local-branch gap; got: %v", err) + } + + // Local branch must still not exist — attach should not have created a + // fresh orphan on refuse. + if _, refErr := repo.Reference(localRef, true); refErr == nil { + t.Error("local entire/checkpoints/v1 branch was created despite refuse; would clobber remote on push") + } + + // Remote-tracking ref must still hold Alice's untouched data. + remoteRef, err := repo.Reference(remoteTrackingRef, true) + if err != nil { + t.Fatalf("remote-tracking ref missing: %v", err) + } + if remoteRef.Hash() != populated.Hash() { + t.Errorf("remote-tracking ref moved from %s to %s", populated.Hash(), remoteRef.Hash()) + } +} + func TestAttach_PopulatesTokenUsage(t *testing.T) { setupAttachTestRepo(t) From 6bf884dc379fbed88e5ca56ccce6516cc76a4290 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Thu, 23 Apr 2026 20:25:42 +0200 Subject: [PATCH 6/9] pin log level in tripwire test The assertion reads the WARN line from the log file, which only gets written when the configured level is WARN or lower. A dev (or CI job) running with ENTIRE_LOG_LEVEL=error would see an empty log and a spurious failure. Setenv the level at the top of the test so the assertion is environment-independent. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 03c1d6bf9b6b --- cmd/entire/cli/checkpoint/committed_tripwire_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmd/entire/cli/checkpoint/committed_tripwire_test.go b/cmd/entire/cli/checkpoint/committed_tripwire_test.go index be82836f66..e85d23807f 100644 --- a/cmd/entire/cli/checkpoint/committed_tripwire_test.go +++ b/cmd/entire/cli/checkpoint/committed_tripwire_test.go @@ -23,6 +23,10 @@ func TestWriteStandardCheckpointEntries_WarnsOnUnexpectedSessionZeroOverwrite(t tmpDir := t.TempDir() t.Chdir(tmpDir) + // Pin log level so the assertion below doesn't depend on a dev/CI + // environment that happens to set ENTIRE_LOG_LEVEL=error. + t.Setenv(logging.LogLevelEnvVar, "warn") + repo, err := git.PlainInit(tmpDir, false) if err != nil { t.Fatalf("PlainInit() error = %v", err) From f114341f232eded8577692ae51e2199baa7140ca Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Thu, 23 Apr 2026 21:17:15 +0200 Subject: [PATCH 7/9] point v2-only attach refuses at the v2 ref MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In v2-only mode (checkpoints_version: 2) the guard was telling users to fetch entire/checkpoints/v1, which is the wrong ref — v2 lives under refs/entire/checkpoints/v2/main. The refresh step also called getMetadataTree (v1) instead of getV2MetadataTree, so the best-effort refresh did nothing useful when v2 was the active format. Branch both the refresh and the fetch-hint on the storage version. v1 stays as-is; v2-only points at refs/entire/checkpoints/v2/main with a fully-qualified refspec (short names don't resolve outside refs/heads/). Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 5ff33f043f7c --- cmd/entire/cli/attach.go | 35 +++++++++++++++++++++------ cmd/entire/cli/attach_test.go | 45 +++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 7 deletions(-) diff --git a/cmd/entire/cli/attach.go b/cmd/entire/cli/attach.go index 413d4d4b21..7af8feccd0 100644 --- a/cmd/entire/cli/attach.go +++ b/cmd/entire/cli/attach.go @@ -284,8 +284,12 @@ func ensureCheckpointAvailable(ctx, logCtx context.Context, repo *git.Repository return repo, nil } - // Missing locally — try to refresh, then re-check. - if _, freshRepo, fetchErr := getMetadataTree(ctx); fetchErr != nil { + // Missing locally — try to refresh, then re-check. Use the same fetch + // chain `entire resume` uses for the active storage version (v2 refs live + // under refs/entire/, not refs/heads/, so v1 and v2 need different + // refspecs). + freshRepo, fetchErr := refreshCheckpointRefs(ctx, v2Only) + if fetchErr != nil { logging.Warn(logCtx, "failed to refresh metadata branch before attach; proceeding with local state", slog.String("error", fetchErr.Error())) } else { @@ -305,10 +309,22 @@ func ensureCheckpointAvailable(ctx, logCtx context.Context, repo *git.Repository } return repo, fmt.Errorf( "checkpoint %s referenced by HEAD is missing from the local %s after a refresh attempt. Creating a fresh checkpoint here would overwrite the original session data on push. Run:\n\n %s\n\nthen re-run attach. If the colleague who made this commit hasn't pushed their checkpoint metadata yet, ask them to do so first", - checkpointID.String(), branchDescription, suggestCheckpointFetchCommand(logCtx), + checkpointID.String(), branchDescription, suggestCheckpointFetchCommand(logCtx, v2Only), ) } +// refreshCheckpointRefs runs the resume-equivalent fetch chain for the storage +// version we're about to write to. Returns a freshly-opened repo so go-git +// sees any newly-fetched packfiles and ref updates. +func refreshCheckpointRefs(ctx context.Context, v2Only bool) (*git.Repository, error) { + if v2Only { + _, repo, err := getV2MetadataTree(ctx) + return repo, err + } + _, repo, err := getMetadataTree(ctx) + return repo, err +} + // checkpointPresentLocally reports whether the checkpoint already exists on // the local ref we would write to. For v1 / dual-write, that's the local // entire/checkpoints/v1 branch (remote-tracking alone is not enough — see @@ -344,10 +360,15 @@ func checkpointPresentLocally(ctx context.Context, repo *git.Repository, checkpo return summary != nil, nil } -// suggestCheckpointFetchCommand returns a git fetch command string the user -// can paste, aware of whether checkpoint_remote is configured. -func suggestCheckpointFetchCommand(ctx context.Context) string { - const ref = "entire/checkpoints/v1:entire/checkpoints/v1" +// suggestCheckpointFetchCommand returns a git fetch command the user can +// paste to pull the missing metadata ref. v2 refs live under refs/entire/ +// (not refs/heads/), so they need an explicit fully-qualified refspec; +// v1 lives on a regular branch and its short name is enough. +func suggestCheckpointFetchCommand(ctx context.Context, v2Only bool) string { + ref := "entire/checkpoints/v1:entire/checkpoints/v1" + if v2Only { + ref = paths.V2MainRefName + ":" + paths.V2MainRefName + } if remote.Configured(ctx) { if url, err := remote.FetchURL(ctx); err == nil && url != "" { return fmt.Sprintf("git fetch %s %s", url, ref) diff --git a/cmd/entire/cli/attach_test.go b/cmd/entire/cli/attach_test.go index 8a5f7a16ab..7146edf542 100644 --- a/cmd/entire/cli/attach_test.go +++ b/cmd/entire/cli/attach_test.go @@ -443,6 +443,39 @@ func TestAttach_RefusesWhenCheckpointOnlyInRemoteTrackingRef(t *testing.T) { } } +// In v2-only mode, the refuse hint must reference the v2 /main ref and +// its fully-qualified refspec (refs/entire/checkpoints/v2/main lives under +// refs/entire/, not refs/heads/, so a short refspec won't resolve). +func TestAttach_RefuseHint_V2Only(t *testing.T) { + setupAttachTestRepo(t) + + repoRoot := mustGetwd(t) + setAttachCheckpointsV2Only(t, repoRoot) + + runGitInDir(t, repoRoot, "commit", "--amend", "-m", "init\n\nEntire-Checkpoint: ffffffffeeee") + + sessionID := "v2-orphaned-attach" + setupClaudeTranscript(t, sessionID, `{"type":"user","message":{"role":"user","content":"hi"},"uuid":"u1"} +`) + + var out bytes.Buffer + err := runAttach(context.Background(), &out, sessionID, agent.AgentNameClaudeCode, true) + if err == nil { + t.Fatal("expected v2-only attach to refuse when checkpoint is missing") + } + if !strings.Contains(err.Error(), "missing from the local v2 /main ref") { + t.Errorf("error should describe the v2 /main ref; got: %v", err) + } + v2Refspec := paths.V2MainRefName + ":" + paths.V2MainRefName + if !strings.Contains(err.Error(), v2Refspec) { + t.Errorf("error should include v2 refspec %q; got: %v", v2Refspec, err) + } + // And must NOT suggest the v1 refspec. + if strings.Contains(err.Error(), "entire/checkpoints/v1:entire/checkpoints/v1") { + t.Errorf("v2-only hint should not reference the v1 branch; got: %v", err) + } +} + func TestAttach_PopulatesTokenUsage(t *testing.T) { setupAttachTestRepo(t) @@ -929,6 +962,18 @@ func setAttachCheckpointsV2Enabled(t *testing.T, repoDir string) { } } +func setAttachCheckpointsV2Only(t *testing.T, repoDir string) { + t.Helper() + entireDir := filepath.Join(repoDir, ".entire") + if err := os.MkdirAll(entireDir, 0o750); err != nil { + t.Fatal(err) + } + settingsContent := `{"enabled": true, "strategy_options": {"checkpoints_version": 2}}` + if err := os.WriteFile(filepath.Join(entireDir, "settings.json"), []byte(settingsContent), 0o600); err != nil { + t.Fatal(err) + } +} + func mustGetwd(t *testing.T) string { t.Helper() dir, err := os.Getwd() From 4332652623daab962d56ddebac6c010ecbace4de Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Thu, 23 Apr 2026 21:52:56 +0200 Subject: [PATCH 8/9] refuse tripwire instead of warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a v1 checkpoint write targets slot 0 and the tree already holds session-0 metadata for a different sessionID, stop instead of logging a warning and proceeding. That shape means either tree corruption or a stale root summary; silently overwriting data we don't know about is the wrong default. findSessionIndex only picks slot 0 when existingSummary is nil or when the summary claims slot 0 is ours, so this check never fires on the normal append-a-session path. In condensation (post-commit hook) the error is swallowed as before — but that path can't produce this shape because condensation always writes under a fresh or summary-coherent ID. The paths that can reach it (attach, migrate) are foreground, so the refuse surfaces to the user. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 8518f599b0e2 --- cmd/entire/cli/checkpoint/committed.go | 38 ++++++++-------- .../cli/checkpoint/committed_tripwire_test.go | 43 +++++++++---------- 2 files changed, 40 insertions(+), 41 deletions(-) diff --git a/cmd/entire/cli/checkpoint/committed.go b/cmd/entire/cli/checkpoint/committed.go index 4e36ffbdfe..23858f76ca 100644 --- a/cmd/entire/cli/checkpoint/committed.go +++ b/cmd/entire/cli/checkpoint/committed.go @@ -317,16 +317,27 @@ func (s *GitStore) writeStandardCheckpointEntries(ctx context.Context, opts Writ // Determine session index: reuse existing slot if session ID matches, otherwise append sessionIndex := s.findSessionIndex(ctx, basePath, existingSummary, entries, opts.SessionID) - // Capture any pre-existing session-0 metadata before writeSessionToSubdirectory - // clears that subtree. The warning below only fires in the suspicious shape - // where findSessionIndex chose slot 0 but the tree already had session-0 - // metadata for a different session — typically meaning the root summary is - // missing/stale while a numbered session subdir still exists. - var existingSessionZeroMeta *CommittedMetadata + // Refuse if slot 0 already holds metadata for a DIFFERENT session ID. + // findSessionIndex only returns 0 when existingSummary is nil (fresh write) + // or when the summary claims slot 0 belongs to us — either way, the tree + // actually holding session-0 metadata for someone else is a corruption / + // stale-summary shape. Writing through it would overwrite data we don't + // know about. Bail instead of silently clobbering. + // + // We read and capture BEFORE writeSessionToSubdirectory clears the subtree, + // otherwise we'd only ever see our own write. if sessionIndex == 0 { if entry, exists := entries[fmt.Sprintf("%s0/%s", basePath, paths.MetadataFileName)]; exists { - if existingMeta, readErr := s.readMetadataFromBlob(entry.Hash); readErr == nil { - existingSessionZeroMeta = existingMeta + if existingMeta, readErr := s.readMetadataFromBlob(entry.Hash); readErr == nil && existingMeta.SessionID != opts.SessionID { + logging.Error(ctx, "refusing checkpoint write: session 0 holds a different sessionID", + slog.String("checkpoint_id", opts.CheckpointID.String()), + slog.String("existing_session_id", existingMeta.SessionID), + slog.String("write_session_id", opts.SessionID), + slog.Bool("existing_summary_nil", existingSummary == nil)) + return fmt.Errorf( + "refusing to overwrite session 0 of checkpoint %s: existing session ID %q differs from write session ID %q (tree corruption or stale summary — run 'entire doctor' to investigate)", + opts.CheckpointID, existingMeta.SessionID, opts.SessionID, + ) } } } @@ -355,17 +366,6 @@ func (s *GitStore) writeStandardCheckpointEntries(ctx context.Context, opts Writ } sessions[sessionIndex] = sessionFilePaths - // Tripwire: if we're writing session 0 and there was already session-0 - // metadata for a DIFFERENT session ID, emit a loud warning. This is a - // tree-corruption / stale-summary shape, not a routine overwrite path. - if existingSessionZeroMeta != nil && existingSessionZeroMeta.SessionID != opts.SessionID { - logging.Warn(ctx, "checkpoint write overwrites session 0 with a different sessionID — potential overwrite regression", - slog.String("checkpoint_id", opts.CheckpointID.String()), - slog.String("existing_session_id", existingSessionZeroMeta.SessionID), - slog.String("write_session_id", opts.SessionID), - slog.Bool("existing_summary_nil", existingSummary == nil)) - } - // Update root metadata.json with CheckpointSummary return s.writeCheckpointSummary(opts, basePath, entries, sessions) } diff --git a/cmd/entire/cli/checkpoint/committed_tripwire_test.go b/cmd/entire/cli/checkpoint/committed_tripwire_test.go index e85d23807f..8baa96ef78 100644 --- a/cmd/entire/cli/checkpoint/committed_tripwire_test.go +++ b/cmd/entire/cli/checkpoint/committed_tripwire_test.go @@ -2,8 +2,6 @@ package checkpoint import ( "context" - "os" - "path/filepath" "strings" "testing" @@ -19,14 +17,10 @@ import ( "github.com/go-git/go-git/v6/plumbing/object" ) -func TestWriteStandardCheckpointEntries_WarnsOnUnexpectedSessionZeroOverwrite(t *testing.T) { +func TestWriteStandardCheckpointEntries_RefusesUnexpectedSessionZeroOverwrite(t *testing.T) { tmpDir := t.TempDir() t.Chdir(tmpDir) - // Pin log level so the assertion below doesn't depend on a dev/CI - // environment that happens to set ENTIRE_LOG_LEVEL=error. - t.Setenv(logging.LogLevelEnvVar, "warn") - repo, err := git.PlainInit(tmpDir, false) if err != nil { t.Fatalf("PlainInit() error = %v", err) @@ -36,6 +30,7 @@ func TestWriteStandardCheckpointEntries_WarnsOnUnexpectedSessionZeroOverwrite(t if err := logging.Init(context.Background(), ""); err != nil { t.Fatalf("logging.Init() error = %v", err) } + defer logging.Close() checkpointID, err := id.Generate() if err != nil { @@ -58,9 +53,10 @@ func TestWriteStandardCheckpointEntries_WarnsOnUnexpectedSessionZeroOverwrite(t t.Fatalf("CreateBlobFromContent(old metadata) error = %v", err) } + sessionZeroPath := basePath + "0/" + paths.MetadataFileName entries := map[string]object.TreeEntry{ - basePath + "0/" + paths.MetadataFileName: { - Name: basePath + "0/" + paths.MetadataFileName, + sessionZeroPath: { + Name: sessionZeroPath, Mode: filemode.Regular, Hash: oldMetadataHash, }, @@ -74,21 +70,24 @@ func TestWriteStandardCheckpointEntries_WarnsOnUnexpectedSessionZeroOverwrite(t Prompts: []string{"hi"}, } - if err := store.writeStandardCheckpointEntries(context.Background(), opts, basePath, entries); err != nil { - t.Fatalf("writeStandardCheckpointEntries() error = %v", err) + err = store.writeStandardCheckpointEntries(context.Background(), opts, basePath, entries) + if err == nil { + t.Fatal("expected writeStandardCheckpointEntries to refuse, got nil error") } - logging.Close() - - logPath := filepath.Join(tmpDir, logging.LogsDir, "entire.log") - logData, err := os.ReadFile(logPath) - if err != nil { - t.Fatalf("ReadFile(%s) error = %v", logPath, err) + if !strings.Contains(err.Error(), "refusing to overwrite session 0") { + t.Errorf("error message should announce the refuse; got: %v", err) } - logText := string(logData) - if !strings.Contains(logText, "checkpoint write overwrites session 0 with a different sessionID") { - t.Fatalf("expected tripwire warning in log, got:\n%s", logText) + if !strings.Contains(err.Error(), "session-old") || !strings.Contains(err.Error(), "session-new") { + t.Errorf("error should include both session IDs; got: %v", err) + } + + // Alice's original metadata must remain untouched in the entries map — + // the refuse runs before writeSessionToSubdirectory clears the subtree. + entry, ok := entries[sessionZeroPath] + if !ok { + t.Fatalf("session 0 metadata entry unexpectedly removed from entries map") } - if !strings.Contains(logText, "session-old") || !strings.Contains(logText, "session-new") { - t.Fatalf("expected log to include both session IDs, got:\n%s", logText) + if entry.Hash != oldMetadataHash { + t.Errorf("session 0 metadata blob changed: got %s, want %s", entry.Hash, oldMetadataHash) } } From aae2449ac860731122b354c3399a5922f8885f87 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Thu, 23 Apr 2026 22:20:07 +0200 Subject: [PATCH 9/9] port session-0 refuse to v2 writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v2 is about to become the primary storage format, and writeMainCheckpointEntries has the exact same clobber shape as v1: if slot 0 already holds metadata for a different sessionID and findSessionIndex still picks 0, writeMainSessionToSubdirectory wipes the subtree and writes ours on top. Mirror the v1 refuse. Drop the "run 'entire doctor' to investigate" breadcrumb from both messages — doctor only checks branch-level disconnection and v2 ref health, not within-tree summary/session-ID coherence. Replace with the git ls-tree invocation the user can actually run to capture the shape. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 664ec1791597 --- cmd/entire/cli/checkpoint/committed.go | 4 +- cmd/entire/cli/checkpoint/v2_committed.go | 23 +++++ .../checkpoint/v2_committed_tripwire_test.go | 87 +++++++++++++++++++ 3 files changed, 112 insertions(+), 2 deletions(-) create mode 100644 cmd/entire/cli/checkpoint/v2_committed_tripwire_test.go diff --git a/cmd/entire/cli/checkpoint/committed.go b/cmd/entire/cli/checkpoint/committed.go index 23858f76ca..75bc491b40 100644 --- a/cmd/entire/cli/checkpoint/committed.go +++ b/cmd/entire/cli/checkpoint/committed.go @@ -335,8 +335,8 @@ func (s *GitStore) writeStandardCheckpointEntries(ctx context.Context, opts Writ slog.String("write_session_id", opts.SessionID), slog.Bool("existing_summary_nil", existingSummary == nil)) return fmt.Errorf( - "refusing to overwrite session 0 of checkpoint %s: existing session ID %q differs from write session ID %q (tree corruption or stale summary — run 'entire doctor' to investigate)", - opts.CheckpointID, existingMeta.SessionID, opts.SessionID, + "refusing to overwrite session 0 of checkpoint %s: existing session ID %q differs from write session ID %q. The checkpoint tree is inconsistent (session 0 belongs to a different session than this write claims). No automated repair exists for this shape — please report it along with the output of `git ls-tree entire/checkpoints/v1 %s/`", + opts.CheckpointID, existingMeta.SessionID, opts.SessionID, opts.CheckpointID.Path(), ) } } diff --git a/cmd/entire/cli/checkpoint/v2_committed.go b/cmd/entire/cli/checkpoint/v2_committed.go index d4f940b9aa..9f63b30a90 100644 --- a/cmd/entire/cli/checkpoint/v2_committed.go +++ b/cmd/entire/cli/checkpoint/v2_committed.go @@ -333,6 +333,29 @@ func (s *V2GitStore) writeMainCheckpointEntries(ctx context.Context, opts WriteC // Determine session index sessionIndex := s.gs.findSessionIndex(ctx, basePath, existingSummary, entries, opts.SessionID) + // Refuse if slot 0 already holds metadata for a DIFFERENT session ID. + // Mirrors GitStore.writeStandardCheckpointEntries: findSessionIndex only + // picks slot 0 when existingSummary is nil or when the summary claims slot 0 + // belongs to us, so the actual tree holding session-0 metadata for someone + // else is a corruption / stale-summary shape. Read BEFORE + // writeMainSessionToSubdirectory clears the subtree, or we'd only ever see + // our own write. + if sessionIndex == 0 { + if entry, exists := entries[fmt.Sprintf("%s0/%s", basePath, paths.MetadataFileName)]; exists { + if existingMeta, readErr := s.gs.readMetadataFromBlob(entry.Hash); readErr == nil && existingMeta.SessionID != opts.SessionID { + logging.Error(ctx, "refusing v2 checkpoint write: session 0 holds a different sessionID", + slog.String("checkpoint_id", opts.CheckpointID.String()), + slog.String("existing_session_id", existingMeta.SessionID), + slog.String("write_session_id", opts.SessionID), + slog.Bool("existing_summary_nil", existingSummary == nil)) + return 0, fmt.Errorf( + "refusing to overwrite session 0 of checkpoint %s: existing session ID %q differs from write session ID %q. The v2 checkpoint tree is inconsistent (session 0 belongs to a different session than this write claims). No automated repair exists for this shape — please report it along with the output of `git ls-tree %s %s/`", + opts.CheckpointID, existingMeta.SessionID, opts.SessionID, paths.V2MainRefName, opts.CheckpointID.Path(), + ) + } + } + } + // Write session files (metadata and prompts — no transcript or content hash) sessionPath := fmt.Sprintf("%s%d/", basePath, sessionIndex) sessionFilePaths, err := s.writeMainSessionToSubdirectory(opts, sessionPath, entries) diff --git a/cmd/entire/cli/checkpoint/v2_committed_tripwire_test.go b/cmd/entire/cli/checkpoint/v2_committed_tripwire_test.go new file mode 100644 index 0000000000..4f2e6e8acd --- /dev/null +++ b/cmd/entire/cli/checkpoint/v2_committed_tripwire_test.go @@ -0,0 +1,87 @@ +package checkpoint + +import ( + "context" + "strings" + "testing" + + "github.com/entireio/cli/cmd/entire/cli/checkpoint/id" + "github.com/entireio/cli/cmd/entire/cli/jsonutil" + "github.com/entireio/cli/cmd/entire/cli/logging" + "github.com/entireio/cli/cmd/entire/cli/paths" + "github.com/entireio/cli/cmd/entire/cli/versioninfo" + + "github.com/go-git/go-git/v6/plumbing/filemode" + "github.com/go-git/go-git/v6/plumbing/object" +) + +// Mirrors TestWriteStandardCheckpointEntries_RefusesUnexpectedSessionZeroOverwrite +// but for the v2 store. Guards writeMainCheckpointEntries against the same +// corruption / stale-summary shape that we catch in v1. +func TestV2WriteMainCheckpointEntries_RefusesUnexpectedSessionZeroOverwrite(t *testing.T) { + repo := initTestRepo(t) + store := NewV2GitStore(repo, "origin") + + if err := logging.Init(context.Background(), ""); err != nil { + t.Fatalf("logging.Init() error = %v", err) + } + defer logging.Close() + + checkpointID, err := id.Generate() + if err != nil { + t.Fatalf("id.Generate() error = %v", err) + } + basePath := checkpointID.Path() + "/" + + oldMetadata := CommittedMetadata{ + CheckpointID: checkpointID, + SessionID: "session-old", + Strategy: "manual-commit", + CLIVersion: versioninfo.Version, + } + oldMetadataJSON, err := jsonutil.MarshalIndentWithNewline(oldMetadata, "", " ") + if err != nil { + t.Fatalf("marshal old metadata: %v", err) + } + oldMetadataHash, err := CreateBlobFromContent(repo, oldMetadataJSON) + if err != nil { + t.Fatalf("CreateBlobFromContent(old metadata) error = %v", err) + } + + sessionZeroPath := basePath + "0/" + paths.MetadataFileName + entries := map[string]object.TreeEntry{ + sessionZeroPath: { + Name: sessionZeroPath, + Mode: filemode.Regular, + Hash: oldMetadataHash, + }, + } + + opts := WriteCommittedOptions{ + CheckpointID: checkpointID, + SessionID: "session-new", + Strategy: "manual-commit", + Prompts: []string{"hi"}, + } + + _, err = store.writeMainCheckpointEntries(context.Background(), opts, basePath, entries) + if err == nil { + t.Fatal("expected writeMainCheckpointEntries to refuse, got nil error") + } + if !strings.Contains(err.Error(), "refusing to overwrite session 0") { + t.Errorf("error message should announce the refuse; got: %v", err) + } + if !strings.Contains(err.Error(), "session-old") || !strings.Contains(err.Error(), "session-new") { + t.Errorf("error should include both session IDs; got: %v", err) + } + + // The original session-0 metadata entry must remain untouched — the refuse + // runs before writeMainSessionToSubdirectory clears the subtree. + entry, ok := entries[sessionZeroPath] + if !ok { + t.Fatalf("session 0 metadata entry unexpectedly removed from entries map") + } + if entry.Hash != oldMetadataHash { + t.Errorf("session 0 metadata blob changed: got %s, want %s", entry.Hash, oldMetadataHash) + } +}