diff --git a/cmd/entire/cli/attach.go b/cmd/entire/cli/attach.go index c2e3a5f713..7af8feccd0 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" ) @@ -79,6 +81,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,6 +138,14 @@ 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) + // 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) @@ -244,6 +255,128 @@ func getHeadCommit(repo *git.Repository) (*object.Commit, error) { return commit, nil } +// 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. +// +// 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 + } + + 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 present { + return repo, 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 { + repo = freshRepo + 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 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 %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, 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 +// 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 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) + } + } + 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). diff --git a/cmd/entire/cli/attach_test.go b/cmd/entire/cli/attach_test.go index 7eac6262b7..7146edf542 100644 --- a/cmd/entire/cli/attach_test.go +++ b/cmd/entire/cli/attach_test.go @@ -18,9 +18,13 @@ 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/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" @@ -255,6 +259,223 @@ 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", "-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) + } +} + +// 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()) + } +} + +// 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) @@ -741,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() @@ -847,3 +1080,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 16f9dfec26..75bc491b40 100644 --- a/cmd/entire/cli/checkpoint/committed.go +++ b/cmd/entire/cli/checkpoint/committed.go @@ -317,6 +317,31 @@ 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) + // 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 && 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. 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(), + ) + } + } + } + // Write session files to numbered subdirectory sessionPath := fmt.Sprintf("%s%d/", basePath, sessionIndex) sessionFilePaths, err := s.writeSessionToSubdirectory(ctx, opts, sessionPath, entries) 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..8baa96ef78 --- /dev/null +++ b/cmd/entire/cli/checkpoint/committed_tripwire_test.go @@ -0,0 +1,93 @@ +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/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_RefusesUnexpectedSessionZeroOverwrite(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) + } + + 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", + Transcript: redact.AlreadyRedacted([]byte("{\"type\":\"user\",\"message\":\"hi\"}\n")), + Prompts: []string{"hi"}, + } + + err = store.writeStandardCheckpointEntries(context.Background(), opts, basePath, entries) + if err == nil { + t.Fatal("expected writeStandardCheckpointEntries 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) + } + + // 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 entry.Hash != oldMetadataHash { + t.Errorf("session 0 metadata blob changed: got %s, want %s", entry.Hash, oldMetadataHash) + } +} 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) + } +}