From bee7df265577c25415a951169e14bb1796cddc66 Mon Sep 17 00:00:00 2001 From: Alex Ong Date: Thu, 19 Feb 2026 15:07:35 +1100 Subject: [PATCH 1/7] fix stale ACTIVE sessions being condensed into every commit When an agent is killed without the Stop hook firing, its session remains in ACTIVE phase permanently. PostCommit unconditionally set hasNew=true for ACTIVE sessions and skipped the file overlap check, so stale ACTIVE sessions got condensed into every subsequent commit. Apply the file overlap check to ALL sessions (including ACTIVE) by intersecting filesTouchedBefore with the actually-committed files before calling filesOverlapWithContent. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 8642696e40dd --- .../cli/strategy/manual_commit_hooks.go | 40 ++++- .../cli/strategy/phase_postcommit_test.go | 164 +++++++++++++++++- 2 files changed, 193 insertions(+), 11 deletions(-) diff --git a/cmd/entire/cli/strategy/manual_commit_hooks.go b/cmd/entire/cli/strategy/manual_commit_hooks.go index 34abea923..e8daf0c38 100644 --- a/cmd/entire/cli/strategy/manual_commit_hooks.go +++ b/cmd/entire/cli/strategy/manual_commit_hooks.go @@ -479,20 +479,45 @@ type postCommitActionHandler struct { shadowBranchesToDelete map[string]struct{} committedFileSet map[string]struct{} hasNew bool + filesTouchedBefore []string // Output: set by handler methods, read by caller after TransitionAndLog. condensed bool } func (h *postCommitActionHandler) HandleCondense(state *session.State) error { - // For ACTIVE sessions, any commit during the turn is session-related. - // For IDLE/ENDED sessions (e.g., carry-forward), also require that the - // committed files overlap with the session's remaining files AND have - // matching content — otherwise an unrelated commit (or a commit with - // completely replaced content) would incorrectly get this session's checkpoint. + // Require that the committed files overlap with the session's files AND + // have matching content — otherwise an unrelated commit would incorrectly + // get this session's checkpoint. + // + // This applies to ALL sessions including ACTIVE ones. A stale ACTIVE session + // (agent killed without Stop hook) would otherwise be condensed into every + // commit because hasNew=true unconditionally for ACTIVE sessions. + // + // filesTouchedBefore is populated from: + // - state.FilesTouched for IDLE/ENDED sessions (set by SaveChanges) + // - transcript extraction for ACTIVE sessions with empty FilesTouched + // + // When filesTouchedBefore is empty (transcript extraction failed + no + // SaveChanges yet), we trust hasNew to avoid data loss for mid-turn commits. shouldCondense := h.hasNew - if shouldCondense && !state.Phase.IsActive() { - shouldCondense = filesOverlapWithContent(h.repo, h.shadowBranchName, h.commit, state.FilesTouched) + if shouldCondense && len(h.filesTouchedBefore) > 0 { + // Only check files that were actually changed in this commit. + // Without this, files that exist in the tree but weren't changed + // would pass the "modified file" check in filesOverlapWithContent + // (because the file exists in the parent tree), causing stale + // sessions to be incorrectly condensed. + var committedTouchedFiles []string + for _, f := range h.filesTouchedBefore { + if _, ok := h.committedFileSet[f]; ok { + committedTouchedFiles = append(committedTouchedFiles, f) + } + } + if len(committedTouchedFiles) > 0 { + shouldCondense = filesOverlapWithContent(h.repo, h.shadowBranchName, h.commit, committedTouchedFiles) + } else { + shouldCondense = false + } } if shouldCondense { h.condensed = h.s.condenseAndUpdateState(h.logCtx, h.repo, h.checkpointID, state, h.head, h.shadowBranchName, h.shadowBranchesToDelete, h.committedFileSet) @@ -654,6 +679,7 @@ func (s *ManualCommitStrategy) PostCommit() error { shadowBranchesToDelete: shadowBranchesToDelete, committedFileSet: committedFileSet, hasNew: hasNew, + filesTouchedBefore: filesTouchedBefore, } if err := TransitionAndLog(state, session.EventGitCommit, transitionCtx, handler); err != nil { fmt.Fprintf(os.Stderr, "[entire] Warning: post-commit action handler error: %v\n", err) diff --git a/cmd/entire/cli/strategy/phase_postcommit_test.go b/cmd/entire/cli/strategy/phase_postcommit_test.go index 330d6e00b..e77866d14 100644 --- a/cmd/entire/cli/strategy/phase_postcommit_test.go +++ b/cmd/entire/cli/strategy/phase_postcommit_test.go @@ -20,6 +20,8 @@ import ( "github.com/stretchr/testify/require" ) +const testNewActiveSessionID = "new-active-session" + // TestPostCommit_ActiveSession_CondensesImmediately verifies that PostCommit on // an ACTIVE session condenses immediately and stays ACTIVE. // With the 1:1 checkpoint model, each commit gets its own checkpoint right away. @@ -1191,9 +1193,24 @@ func TestHandleTurnEnd_PartialFailure(t *testing.T) { commitWithCheckpointTrailer(t, repo, dir, "a1b2c3d4e5f6") require.NoError(t, s.PostCommit()) - // Write new content and create a second real checkpoint + // Write new content and create a second checkpoint on the shadow branch. + // Use SaveChanges directly (instead of setupSessionWithCheckpoint) so that + // second.txt is included in FilesTouched — the overlap check needs it. require.NoError(t, os.WriteFile(filepath.Join(dir, "second.txt"), []byte("second file"), 0o644)) - setupSessionWithCheckpoint(t, s, repo, dir, sessionID) // refresh shadow branch + metadataDir := ".entire/metadata/" + sessionID + metadataDirAbs := filepath.Join(dir, metadataDir) + err = s.SaveChanges(SaveContext{ + SessionID: sessionID, + ModifiedFiles: []string{"test.txt"}, + NewFiles: []string{"second.txt"}, + DeletedFiles: []string{}, + MetadataDir: metadataDir, + MetadataDirAbs: metadataDirAbs, + CommitMessage: "Checkpoint 2", + AuthorName: "Test", + AuthorEmail: "test@test.com", + }) + require.NoError(t, err, "SaveChanges should succeed for second checkpoint") state, err = s.loadSessionState(sessionID) require.NoError(t, err) state.Phase = session.PhaseActive @@ -1370,7 +1387,7 @@ func TestPostCommit_OldIdleSession_BaseCommitNotUpdated(t *testing.T) { require.NoError(t, err) // --- Create a NEW ACTIVE session at the new HEAD --- - newSessionID := "new-active-session" + newSessionID := testNewActiveSessionID setupSessionWithCheckpoint(t, s, repo, dir, newSessionID) newState, err := s.loadSessionState(newSessionID) @@ -1453,7 +1470,7 @@ func TestPostCommit_OldEndedSession_BaseCommitNotUpdated(t *testing.T) { require.NoError(t, err) // --- Create a NEW ACTIVE session at the new HEAD --- - newSessionID := "new-active-session" + newSessionID := testNewActiveSessionID setupSessionWithCheckpoint(t, s, repo, dir, newSessionID) newState, err := s.loadSessionState(newSessionID) @@ -1487,3 +1504,142 @@ func TestPostCommit_OldEndedSession_BaseCommitNotUpdated(t *testing.T) { assert.Equal(t, newHead, newState.BaseCommit, "NEW ACTIVE session's BaseCommit should be updated after condensation") } + +// TestPostCommit_StaleActiveSession_NotCondensed verifies that a stale ACTIVE +// session (agent killed without Stop hook) is NOT condensed into an unrelated +// commit from a different session. +// +// Root cause: when an agent is killed without the Stop hook firing, its session +// remains in ACTIVE phase permanently. Previously, PostCommit unconditionally +// set hasNew=true for ACTIVE sessions and skipped the filesOverlapWithContent +// check, so stale ACTIVE sessions got condensed into every commit. +// +// The fix applies the overlap check to ALL sessions (including ACTIVE) using +// filesTouchedBefore, so stale sessions with unrelated files are filtered out. +func TestPostCommit_StaleActiveSession_NotCondensed(t *testing.T) { + dir := setupGitRepo(t) + t.Chdir(dir) + + repo, err := git.PlainOpen(dir) + require.NoError(t, err) + + s := &ManualCommitStrategy{} + + // --- Create a stale ACTIVE session from an old commit --- + // This simulates an agent that was killed without the Stop hook firing. + staleSessionID := "stale-active-session" + setupSessionWithCheckpoint(t, s, repo, dir, staleSessionID) + + staleState, err := s.loadSessionState(staleSessionID) + require.NoError(t, err) + staleState.Phase = session.PhaseActive + // The stale session touched "test.txt" (set by setupSessionWithCheckpoint) + // but the new commit will modify a different file. + staleState.FilesTouched = []string{"test.txt"} + require.NoError(t, s.saveSessionState(staleState)) + + staleOriginalBaseCommit := staleState.BaseCommit + staleOriginalStepCount := staleState.StepCount + + // Move HEAD forward with an unrelated commit (no trailer) + wt, err := repo.Worktree() + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(dir, "unrelated.txt"), []byte("unrelated work"), 0o644)) + _, err = wt.Add("unrelated.txt") + require.NoError(t, err) + _, err = wt.Commit("unrelated commit", &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@test.com", When: time.Now()}, + }) + require.NoError(t, err) + + // --- Create a NEW ACTIVE session at the new HEAD --- + newSessionID := testNewActiveSessionID + + // Create a new file for the new session (different from stale session's test.txt) + require.NoError(t, os.WriteFile(filepath.Join(dir, "new-feature.txt"), []byte("new feature content"), 0o644)) + + metadataDir := ".entire/metadata/" + newSessionID + metadataDirAbs := filepath.Join(dir, metadataDir) + require.NoError(t, os.MkdirAll(metadataDirAbs, 0o755)) + + transcript := `{"type":"human","message":{"content":"add new feature"}} +{"type":"assistant","message":{"content":"adding new feature"}} +` + require.NoError(t, os.WriteFile( + filepath.Join(metadataDirAbs, paths.TranscriptFileName), + []byte(transcript), 0o644)) + + err = s.SaveChanges(SaveContext{ + SessionID: newSessionID, + ModifiedFiles: []string{}, + NewFiles: []string{"new-feature.txt"}, + DeletedFiles: []string{}, + MetadataDir: metadataDir, + MetadataDirAbs: metadataDirAbs, + CommitMessage: "Checkpoint: new feature", + AuthorName: "Test", + AuthorEmail: "test@test.com", + }) + require.NoError(t, err) + + newState, err := s.loadSessionState(newSessionID) + require.NoError(t, err) + newState.Phase = session.PhaseActive + require.NoError(t, s.saveSessionState(newState)) + + // --- Commit ONLY new-feature.txt (not test.txt) with checkpoint trailer --- + wt, err = repo.Worktree() + require.NoError(t, err) + _, err = wt.Add("new-feature.txt") + require.NoError(t, err) + + cpID := "de1de2de3de4" + commitMsg := "add new feature\n\n" + trailers.CheckpointTrailerKey + ": " + cpID + "\n" + _, err = wt.Commit(commitMsg, &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@test.com", When: time.Now()}, + }) + require.NoError(t, err) + + head, err := repo.Head() + require.NoError(t, err) + newHead := head.Hash().String() + + // Run PostCommit + err = s.PostCommit() + require.NoError(t, err) + + // --- Verify: stale ACTIVE session was NOT condensed --- + staleState, err = s.loadSessionState(staleSessionID) + require.NoError(t, err) + + // StepCount should be unchanged (not reset by condensation) + assert.Equal(t, staleOriginalStepCount, staleState.StepCount, + "Stale ACTIVE session StepCount should NOT be reset (no condensation)") + + // BaseCommit IS updated for ACTIVE sessions (updateBaseCommitIfChanged) + assert.Equal(t, newHead, staleState.BaseCommit, + "Stale ACTIVE session BaseCommit should be updated (ACTIVE sessions always get BaseCommit updated)") + assert.NotEqual(t, staleOriginalBaseCommit, staleState.BaseCommit, + "Stale ACTIVE session BaseCommit should have changed") + + // Phase stays ACTIVE + assert.Equal(t, session.PhaseActive, staleState.Phase, + "Stale ACTIVE session should remain ACTIVE") + + // --- Verify: new ACTIVE session WAS condensed --- + newState, err = s.loadSessionState(newSessionID) + require.NoError(t, err) + + // StepCount reset to 0 by condensation + assert.Equal(t, 0, newState.StepCount, + "New ACTIVE session StepCount should be reset by condensation") + + // BaseCommit updated to new HEAD + assert.Equal(t, newHead, newState.BaseCommit, + "New ACTIVE session BaseCommit should be updated after condensation") + + // Verify entire/checkpoints/v1 exists (new session was condensed) + _, err = repo.Reference(plumbing.NewBranchReferenceName(paths.MetadataBranchName), true) + require.NoError(t, err, + "entire/checkpoints/v1 should exist (new session was condensed)") +} From 744c4f3fa48a48fc027ff1951155f9b0e3c0cdc0 Mon Sep 17 00:00:00 2001 From: Alex Ong Date: Thu, 19 Feb 2026 16:02:42 +1100 Subject: [PATCH 2/7] fix ENDED sessions with carry-forward re-condensed into every commit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The overlap check from the previous commit only applied to HandleCondense (ACTIVE/IDLE sessions). ENDED sessions go through HandleCondenseIfFilesTouched, which had no overlap check at all — it only checked len(FilesTouched) > 0. Carry-forward sets FilesTouched with remaining uncommitted files after condensation. On the next commit, sessionHasNewContent returns true (shadow branch has content), and HandleCondenseIfFilesTouched condenses the session again. This cycle repeats on every commit indefinitely. Extract shouldCondenseWithOverlapCheck as a shared method and use it in both HandleCondense and HandleCondenseIfFilesTouched. This ensures ALL condensation paths verify that committed files actually overlap with session files. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 2fbfa77cb815 --- .../cli/strategy/manual_commit_hooks.go | 75 +++++----- .../cli/strategy/phase_postcommit_test.go | 130 ++++++++++++++++++ 2 files changed, 170 insertions(+), 35 deletions(-) diff --git a/cmd/entire/cli/strategy/manual_commit_hooks.go b/cmd/entire/cli/strategy/manual_commit_hooks.go index e8daf0c38..cbcdf3a56 100644 --- a/cmd/entire/cli/strategy/manual_commit_hooks.go +++ b/cmd/entire/cli/strategy/manual_commit_hooks.go @@ -486,40 +486,7 @@ type postCommitActionHandler struct { } func (h *postCommitActionHandler) HandleCondense(state *session.State) error { - // Require that the committed files overlap with the session's files AND - // have matching content — otherwise an unrelated commit would incorrectly - // get this session's checkpoint. - // - // This applies to ALL sessions including ACTIVE ones. A stale ACTIVE session - // (agent killed without Stop hook) would otherwise be condensed into every - // commit because hasNew=true unconditionally for ACTIVE sessions. - // - // filesTouchedBefore is populated from: - // - state.FilesTouched for IDLE/ENDED sessions (set by SaveChanges) - // - transcript extraction for ACTIVE sessions with empty FilesTouched - // - // When filesTouchedBefore is empty (transcript extraction failed + no - // SaveChanges yet), we trust hasNew to avoid data loss for mid-turn commits. - shouldCondense := h.hasNew - if shouldCondense && len(h.filesTouchedBefore) > 0 { - // Only check files that were actually changed in this commit. - // Without this, files that exist in the tree but weren't changed - // would pass the "modified file" check in filesOverlapWithContent - // (because the file exists in the parent tree), causing stale - // sessions to be incorrectly condensed. - var committedTouchedFiles []string - for _, f := range h.filesTouchedBefore { - if _, ok := h.committedFileSet[f]; ok { - committedTouchedFiles = append(committedTouchedFiles, f) - } - } - if len(committedTouchedFiles) > 0 { - shouldCondense = filesOverlapWithContent(h.repo, h.shadowBranchName, h.commit, committedTouchedFiles) - } else { - shouldCondense = false - } - } - if shouldCondense { + if h.shouldCondenseWithOverlapCheck() { h.condensed = h.s.condenseAndUpdateState(h.logCtx, h.repo, h.checkpointID, state, h.head, h.shadowBranchName, h.shadowBranchesToDelete, h.committedFileSet) } else { h.s.updateBaseCommitIfChanged(h.logCtx, state, h.newHead) @@ -528,7 +495,7 @@ func (h *postCommitActionHandler) HandleCondense(state *session.State) error { } func (h *postCommitActionHandler) HandleCondenseIfFilesTouched(state *session.State) error { - if len(state.FilesTouched) > 0 && h.hasNew { + if len(state.FilesTouched) > 0 && h.shouldCondenseWithOverlapCheck() { h.condensed = h.s.condenseAndUpdateState(h.logCtx, h.repo, h.checkpointID, state, h.head, h.shadowBranchName, h.shadowBranchesToDelete, h.committedFileSet) } else { h.s.updateBaseCommitIfChanged(h.logCtx, state, h.newHead) @@ -536,6 +503,44 @@ func (h *postCommitActionHandler) HandleCondenseIfFilesTouched(state *session.St return nil } +// shouldCondenseWithOverlapCheck returns true if the session should be condensed +// into this commit. Requires both that hasNew is true AND that the session's files +// overlap with the committed files with matching content. +// +// This prevents stale sessions (ACTIVE sessions where the agent was killed, or +// ENDED/IDLE sessions with carry-forward files) from being condensed into every +// unrelated commit. +// +// filesTouchedBefore is populated from: +// - state.FilesTouched for IDLE/ENDED sessions (set by SaveChanges) +// - transcript extraction for ACTIVE sessions with empty FilesTouched +// +// When filesTouchedBefore is empty (transcript extraction failed + no SaveChanges +// yet), we trust hasNew to avoid data loss for mid-turn commits. +func (h *postCommitActionHandler) shouldCondenseWithOverlapCheck() bool { + if !h.hasNew { + return false + } + if len(h.filesTouchedBefore) == 0 { + return true // Fail-open: no file list to check against + } + // Only check files that were actually changed in this commit. + // Without this, files that exist in the tree but weren't changed + // would pass the "modified file" check in filesOverlapWithContent + // (because the file exists in the parent tree), causing stale + // sessions to be incorrectly condensed. + var committedTouchedFiles []string + for _, f := range h.filesTouchedBefore { + if _, ok := h.committedFileSet[f]; ok { + committedTouchedFiles = append(committedTouchedFiles, f) + } + } + if len(committedTouchedFiles) == 0 { + return false + } + return filesOverlapWithContent(h.repo, h.shadowBranchName, h.commit, committedTouchedFiles) +} + func (h *postCommitActionHandler) HandleDiscardIfNoFiles(state *session.State) error { if len(state.FilesTouched) == 0 { logging.Debug(h.logCtx, "post-commit: skipping empty ended session (no files to condense)", diff --git a/cmd/entire/cli/strategy/phase_postcommit_test.go b/cmd/entire/cli/strategy/phase_postcommit_test.go index e77866d14..9f7e33123 100644 --- a/cmd/entire/cli/strategy/phase_postcommit_test.go +++ b/cmd/entire/cli/strategy/phase_postcommit_test.go @@ -1505,6 +1505,136 @@ func TestPostCommit_OldEndedSession_BaseCommitNotUpdated(t *testing.T) { "NEW ACTIVE session's BaseCommit should be updated after condensation") } +// TestPostCommit_EndedSessionCarryForward_NotCondensedIntoUnrelatedCommit verifies +// that an ENDED session with carry-forward files is NOT condensed into a commit +// that doesn't touch any of those files. +// +// This is the primary bug scenario: ENDED sessions go through HandleCondenseIfFilesTouched, +// which previously only checked len(FilesTouched) > 0 && hasNew — no overlap check. +// Carry-forward would set FilesTouched with remaining uncommitted files, and +// sessionHasNewContent returned true because the shadow branch had content. This +// caused ENDED sessions to be re-condensed into every subsequent commit indefinitely. +func TestPostCommit_EndedSessionCarryForward_NotCondensedIntoUnrelatedCommit(t *testing.T) { + dir := setupGitRepo(t) + t.Chdir(dir) + + repo, err := git.PlainOpen(dir) + require.NoError(t, err) + + s := &ManualCommitStrategy{} + + // --- Create an ENDED session with carry-forward files --- + endedSessionID := "ended-carry-forward" + setupSessionWithCheckpoint(t, s, repo, dir, endedSessionID) + + endedState, err := s.loadSessionState(endedSessionID) + require.NoError(t, err) + now := time.Now() + endedState.Phase = session.PhaseEnded + endedState.EndedAt = &now + // Simulate carry-forward: session touched test.txt but it wasn't fully committed yet. + // CheckpointTranscriptStart=0 so sessionHasNewContent returns true (transcript grew). + endedState.FilesTouched = []string{"test.txt"} + endedState.CheckpointTranscriptStart = 0 + require.NoError(t, s.saveSessionState(endedState)) + + endedOriginalBaseCommit := endedState.BaseCommit + endedOriginalStepCount := endedState.StepCount + + // Move HEAD forward with an unrelated commit (no trailer) + wt, err := repo.Worktree() + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(dir, "unrelated.txt"), []byte("unrelated work"), 0o644)) + _, err = wt.Add("unrelated.txt") + require.NoError(t, err) + _, err = wt.Commit("unrelated commit", &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@test.com", When: time.Now()}, + }) + require.NoError(t, err) + + // --- Create a NEW ACTIVE session at the new HEAD --- + newSessionID := testNewActiveSessionID + require.NoError(t, os.WriteFile(filepath.Join(dir, "new-feature.txt"), []byte("new feature content"), 0o644)) + + metadataDir := ".entire/metadata/" + newSessionID + metadataDirAbs := filepath.Join(dir, metadataDir) + require.NoError(t, os.MkdirAll(metadataDirAbs, 0o755)) + + transcript := `{"type":"human","message":{"content":"add new feature"}} +{"type":"assistant","message":{"content":"adding new feature"}} +` + require.NoError(t, os.WriteFile( + filepath.Join(metadataDirAbs, paths.TranscriptFileName), + []byte(transcript), 0o644)) + + err = s.SaveChanges(SaveContext{ + SessionID: newSessionID, + ModifiedFiles: []string{}, + NewFiles: []string{"new-feature.txt"}, + DeletedFiles: []string{}, + MetadataDir: metadataDir, + MetadataDirAbs: metadataDirAbs, + CommitMessage: "Checkpoint: new feature", + AuthorName: "Test", + AuthorEmail: "test@test.com", + }) + require.NoError(t, err) + + newState, err := s.loadSessionState(newSessionID) + require.NoError(t, err) + newState.Phase = session.PhaseActive + require.NoError(t, s.saveSessionState(newState)) + + // --- Commit ONLY new-feature.txt (not test.txt) with checkpoint trailer --- + wt, err = repo.Worktree() + require.NoError(t, err) + _, err = wt.Add("new-feature.txt") + require.NoError(t, err) + + cpID := "ae1ae2ae3ae4" + commitMsg := "add new feature\n\n" + trailers.CheckpointTrailerKey + ": " + cpID + "\n" + _, err = wt.Commit(commitMsg, &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@test.com", When: time.Now()}, + }) + require.NoError(t, err) + + head, err := repo.Head() + require.NoError(t, err) + newHead := head.Hash().String() + + // Run PostCommit + err = s.PostCommit() + require.NoError(t, err) + + // --- Verify: ENDED session was NOT condensed --- + endedState, err = s.loadSessionState(endedSessionID) + require.NoError(t, err) + + // StepCount should be unchanged (not reset by condensation) + assert.Equal(t, endedOriginalStepCount, endedState.StepCount, + "ENDED session StepCount should NOT be reset (no condensation)") + + // BaseCommit should NOT be updated for ENDED sessions (PR #359) + assert.Equal(t, endedOriginalBaseCommit, endedState.BaseCommit, + "ENDED session BaseCommit should NOT be updated") + + // FilesTouched should still have the carry-forward files (not cleared by condensation) + assert.Equal(t, []string{"test.txt"}, endedState.FilesTouched, + "ENDED session FilesTouched should be preserved (carry-forward files not consumed)") + + // Phase stays ENDED + assert.Equal(t, session.PhaseEnded, endedState.Phase, + "ENDED session should remain ENDED") + + // --- Verify: new ACTIVE session WAS condensed --- + newState, err = s.loadSessionState(newSessionID) + require.NoError(t, err) + assert.Equal(t, 0, newState.StepCount, + "New ACTIVE session StepCount should be reset by condensation") + assert.Equal(t, newHead, newState.BaseCommit, + "New ACTIVE session BaseCommit should be updated after condensation") +} + // TestPostCommit_StaleActiveSession_NotCondensed verifies that a stale ACTIVE // session (agent killed without Stop hook) is NOT condensed into an unrelated // commit from a different session. From 014f53f22e98c165817337a8fec6f2dae6702403 Mon Sep 17 00:00:00 2001 From: Alex Ong Date: Thu, 19 Feb 2026 16:42:05 +1100 Subject: [PATCH 3/7] fix IDLE sessions with empty FilesTouched incorrectly condensed shouldCondenseWithOverlapCheck() was fail-open when filesTouchedBefore was empty, causing IDLE sessions with no files (e.g., conversation-only sessions) to be condensed into unrelated commits. Now only ACTIVE sessions fail-open (to handle mid-turn commits before files are saved to state); IDLE/ENDED sessions with no files return false. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 058ec85be074 --- .../cli/strategy/manual_commit_hooks.go | 15 +++-- .../cli/strategy/phase_postcommit_test.go | 65 +++++++++++++++++++ 2 files changed, 74 insertions(+), 6 deletions(-) diff --git a/cmd/entire/cli/strategy/manual_commit_hooks.go b/cmd/entire/cli/strategy/manual_commit_hooks.go index e1b8317c9..ba5f99c03 100644 --- a/cmd/entire/cli/strategy/manual_commit_hooks.go +++ b/cmd/entire/cli/strategy/manual_commit_hooks.go @@ -486,7 +486,7 @@ type postCommitActionHandler struct { } func (h *postCommitActionHandler) HandleCondense(state *session.State) error { - shouldCondense := h.shouldCondenseWithOverlapCheck() + shouldCondense := h.shouldCondenseWithOverlapCheck(state.Phase.IsActive()) logging.Debug(h.logCtx, "post-commit: HandleCondense decision", slog.String("session_id", state.SessionID), @@ -505,7 +505,7 @@ func (h *postCommitActionHandler) HandleCondense(state *session.State) error { } func (h *postCommitActionHandler) HandleCondenseIfFilesTouched(state *session.State) error { - shouldCondense := len(state.FilesTouched) > 0 && h.shouldCondenseWithOverlapCheck() + shouldCondense := len(state.FilesTouched) > 0 && h.shouldCondenseWithOverlapCheck(state.Phase.IsActive()) logging.Debug(h.logCtx, "post-commit: HandleCondenseIfFilesTouched decision", slog.String("session_id", state.SessionID), @@ -536,14 +536,17 @@ func (h *postCommitActionHandler) HandleCondenseIfFilesTouched(state *session.St // - state.FilesTouched for IDLE/ENDED sessions (set by SaveChanges) // - transcript extraction for ACTIVE sessions with empty FilesTouched // -// When filesTouchedBefore is empty (transcript extraction failed + no SaveChanges -// yet), we trust hasNew to avoid data loss for mid-turn commits. -func (h *postCommitActionHandler) shouldCondenseWithOverlapCheck() bool { +// When filesTouchedBefore is empty: +// - For ACTIVE sessions: fail-open (trust hasNew) because the agent may be +// mid-turn before any files are saved to state. +// - For IDLE/ENDED sessions: return false because there are no files to +// overlap with the commit. +func (h *postCommitActionHandler) shouldCondenseWithOverlapCheck(isActive bool) bool { if !h.hasNew { return false } if len(h.filesTouchedBefore) == 0 { - return true // Fail-open: no file list to check against + return isActive // ACTIVE: fail-open; IDLE/ENDED: no files = no overlap } // Only check files that were actually changed in this commit. // Without this, files that exist in the tree but weren't changed diff --git a/cmd/entire/cli/strategy/phase_postcommit_test.go b/cmd/entire/cli/strategy/phase_postcommit_test.go index 761c7d8c1..66a123ffe 100644 --- a/cmd/entire/cli/strategy/phase_postcommit_test.go +++ b/cmd/entire/cli/strategy/phase_postcommit_test.go @@ -1773,3 +1773,68 @@ func TestPostCommit_StaleActiveSession_NotCondensed(t *testing.T) { require.NoError(t, err, "entire/checkpoints/v1 should exist (new session was condensed)") } + +// TestPostCommit_IdleSessionEmptyFilesTouched_NotCondensed verifies that an IDLE +// session with hasNew=true but empty FilesTouched is NOT condensed into a commit. +// +// This can happen for conversation-only sessions where the transcript grew but no +// files were modified. Previously, filesOverlapWithContent was called with an empty +// list and returned false. The shouldCondenseWithOverlapCheck method must also +// return false when filesTouchedBefore is empty. +func TestPostCommit_IdleSessionEmptyFilesTouched_NotCondensed(t *testing.T) { + dir := setupGitRepo(t) + t.Chdir(dir) + + repo, err := git.PlainOpen(dir) + require.NoError(t, err) + + s := &ManualCommitStrategy{} + + // --- Create an IDLE session with a checkpoint but no files touched --- + idleSessionID := "idle-no-files-session" + setupSessionWithCheckpoint(t, s, repo, dir, idleSessionID) + + idleState, err := s.loadSessionState(idleSessionID) + require.NoError(t, err) + idleState.Phase = session.PhaseIdle + // Clear FilesTouched to simulate a conversation-only session + idleState.FilesTouched = nil + // CheckpointTranscriptStart=0 so sessionHasNewContent returns true + idleState.CheckpointTranscriptStart = 0 + require.NoError(t, s.saveSessionState(idleState)) + + idleOriginalStepCount := idleState.StepCount + + // --- Make a commit with an unrelated file --- + wt, err := repo.Worktree() + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(dir, "other-work.txt"), []byte("other work"), 0o644)) + _, err = wt.Add("other-work.txt") + require.NoError(t, err) + + cpID := "f1f2f3f4f5f6" + commitMsg := "other work\n\n" + trailers.CheckpointTrailerKey + ": " + cpID + "\n" + _, err = wt.Commit(commitMsg, &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@test.com", When: time.Now()}, + }) + require.NoError(t, err) + + head, err := repo.Head() + require.NoError(t, err) + newHead := head.Hash().String() + + // Run PostCommit + err = s.PostCommit() + require.NoError(t, err) + + // --- Verify: IDLE session with no files was NOT condensed --- + idleState, err = s.loadSessionState(idleSessionID) + require.NoError(t, err) + + assert.Equal(t, idleOriginalStepCount, idleState.StepCount, + "IDLE session with empty FilesTouched should NOT be condensed") + assert.Equal(t, session.PhaseIdle, idleState.Phase, + "IDLE session should remain IDLE") + // BaseCommit is NOT updated for non-ACTIVE sessions (updateBaseCommitIfChanged skips them) + _ = newHead +} From 55ea754681388d1a76fba774e793f38333202f74 Mon Sep 17 00:00:00 2001 From: Alex Ong Date: Thu, 19 Feb 2026 17:05:33 +1100 Subject: [PATCH 4/7] address Copilot review: fix stale comment and remove unused var - Update comment to reference SaveStep/SaveTaskStep instead of nonexistent SaveChanges - Remove unused newHead computation in idle empty-files test Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 2ac81686c7e4 --- cmd/entire/cli/strategy/manual_commit_hooks.go | 4 ++-- cmd/entire/cli/strategy/phase_postcommit_test.go | 5 ----- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/cmd/entire/cli/strategy/manual_commit_hooks.go b/cmd/entire/cli/strategy/manual_commit_hooks.go index ba5f99c03..9fd817efe 100644 --- a/cmd/entire/cli/strategy/manual_commit_hooks.go +++ b/cmd/entire/cli/strategy/manual_commit_hooks.go @@ -533,8 +533,8 @@ func (h *postCommitActionHandler) HandleCondenseIfFilesTouched(state *session.St // unrelated commit. // // filesTouchedBefore is populated from: -// - state.FilesTouched for IDLE/ENDED sessions (set by SaveChanges) -// - transcript extraction for ACTIVE sessions with empty FilesTouched +// - state.FilesTouched for IDLE/ENDED sessions (set via SaveStep/SaveTaskStep -> mergeFilesTouched) +// - transcript extraction for ACTIVE sessions when FilesTouched is empty // // When filesTouchedBefore is empty: // - For ACTIVE sessions: fail-open (trust hasNew) because the agent may be diff --git a/cmd/entire/cli/strategy/phase_postcommit_test.go b/cmd/entire/cli/strategy/phase_postcommit_test.go index 66a123ffe..a02fdc32c 100644 --- a/cmd/entire/cli/strategy/phase_postcommit_test.go +++ b/cmd/entire/cli/strategy/phase_postcommit_test.go @@ -1819,10 +1819,6 @@ func TestPostCommit_IdleSessionEmptyFilesTouched_NotCondensed(t *testing.T) { }) require.NoError(t, err) - head, err := repo.Head() - require.NoError(t, err) - newHead := head.Hash().String() - // Run PostCommit err = s.PostCommit() require.NoError(t, err) @@ -1836,5 +1832,4 @@ func TestPostCommit_IdleSessionEmptyFilesTouched_NotCondensed(t *testing.T) { assert.Equal(t, session.PhaseIdle, idleState.Phase, "IDLE session should remain IDLE") // BaseCommit is NOT updated for non-ACTIVE sessions (updateBaseCommitIfChanged skips them) - _ = newHead } From 40af1a06da390adef2f165e6ae48b4c17d7cb5bb Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Thu, 19 Feb 2026 11:19:16 +0100 Subject: [PATCH 5/7] fixed another issue assigning wrong files to sessions, added more tests Entire-Checkpoint: 18029dc76a14 --- .../carry_forward_overlap_test.go | 893 ++++++++++++++++++ .../mid_session_commit_test.go | 75 ++ .../strategy/manual_commit_condensation.go | 17 +- cmd/entire/cli/strategy/manual_commit_test.go | 230 +++++ 4 files changed, 1210 insertions(+), 5 deletions(-) create mode 100644 cmd/entire/cli/integration_test/carry_forward_overlap_test.go diff --git a/cmd/entire/cli/integration_test/carry_forward_overlap_test.go b/cmd/entire/cli/integration_test/carry_forward_overlap_test.go new file mode 100644 index 000000000..858650fb5 --- /dev/null +++ b/cmd/entire/cli/integration_test/carry_forward_overlap_test.go @@ -0,0 +1,893 @@ +//go:build integration + +package integration + +import ( + "testing" + + "github.com/entireio/cli/cmd/entire/cli/session" + "github.com/entireio/cli/cmd/entire/cli/strategy" +) + +// TestCarryForward_NotCondensedIntoUnrelatedCommit verifies that a session with +// carry-forward files is NOT condensed into a commit that doesn't touch those files. +// +// This is a regression test for the bug where sessions with carry-forward files +// would be re-condensed into every subsequent commit indefinitely. The root cause +// was that HandleCondense and HandleCondenseIfFilesTouched only checked hasNew +// (shadow branch has content) but didn't verify that the committed files actually +// overlapped with the session's FilesTouched. +// +// Scenario: +// 1. Session 1 creates file1.txt and file2.txt +// 2. User commits ONLY file1.txt (partial commit) +// 3. Session 1 gets carry-forward: FilesTouched = ["file2.txt"] +// 4. Session 1 ends +// 5. New session 2 creates file3.txt (unrelated to session 1) +// 6. Session 2 commits file3.txt +// 7. Verify: Session 1 was NOT condensed (FilesTouched preserved, StepCount unchanged) +func TestCarryForward_NotCondensedIntoUnrelatedCommit(t *testing.T) { + t.Parallel() + env := NewTestEnv(t) + defer env.Cleanup() + + // ======================================== + // Setup + // ======================================== + env.InitRepo() + env.WriteFile("README.md", "# Test Repository") + env.GitAdd("README.md") + env.GitCommit("Initial commit") + env.GitCheckoutNewBranch("feature/carry-forward-test") + env.InitEntire(strategy.StrategyNameManualCommit) + + // ======================================== + // Phase 1: Session 1 creates multiple files + // ======================================== + t.Log("Phase 1: Session 1 creates file1.txt and file2.txt") + + session1 := env.NewSession() + if err := env.SimulateUserPromptSubmit(session1.ID); err != nil { + t.Fatalf("SimulateUserPromptSubmit for session1 failed: %v", err) + } + + // Create two files + env.WriteFile("file1.txt", "content from session 1 - file 1") + env.WriteFile("file2.txt", "content from session 1 - file 2") + session1.CreateTranscript( + "Create file1 and file2", + []FileChange{ + {Path: "file1.txt", Content: "content from session 1 - file 1"}, + {Path: "file2.txt", Content: "content from session 1 - file 2"}, + }, + ) + if err := env.SimulateStop(session1.ID, session1.TranscriptPath); err != nil { + t.Fatalf("SimulateStop for session1 failed: %v", err) + } + + // Verify session1 is IDLE with both files in FilesTouched + state1, err := env.GetSessionState(session1.ID) + if err != nil { + t.Fatalf("GetSessionState for session1 failed: %v", err) + } + if state1.Phase != session.PhaseIdle { + t.Fatalf("Expected session1 to be IDLE, got %s", state1.Phase) + } + t.Logf("Session1 FilesTouched before partial commit: %v", state1.FilesTouched) + + // ======================================== + // Phase 2: Partial commit - only file1.txt + // ======================================== + t.Log("Phase 2: Committing only file1.txt (partial commit)") + + env.GitAdd("file1.txt") + // Note: file2.txt is NOT staged + env.GitCommitWithShadowHooks("Partial commit: only file1", "file1.txt") + + // Verify carry-forward: file2.txt should remain in FilesTouched + state1, err = env.GetSessionState(session1.ID) + if err != nil { + t.Fatalf("GetSessionState for session1 after partial commit failed: %v", err) + } + t.Logf("Session1 FilesTouched after partial commit: %v", state1.FilesTouched) + + // file2.txt should be carried forward (not committed yet) + hasFile2 := false + for _, f := range state1.FilesTouched { + if f == "file2.txt" { + hasFile2 = true + break + } + } + if !hasFile2 { + t.Fatalf("Expected file2.txt to be carried forward in FilesTouched, got: %v", state1.FilesTouched) + } + + // Record state for later comparison + session1StepCountAfterPartial := state1.StepCount + session1BaseCommitAfterPartial := state1.BaseCommit + + // ======================================== + // Phase 3: End session 1 (simulating user closes agent) + // ======================================== + t.Log("Phase 3: Ending session 1") + + state1.Phase = session.PhaseEnded + if err := env.WriteSessionState(session1.ID, state1); err != nil { + t.Fatalf("WriteSessionState for session1 failed: %v", err) + } + + // ======================================== + // Phase 4: New session 2 creates unrelated file + // ======================================== + t.Log("Phase 4: Session 2 creates file3.txt (unrelated to session 1)") + + session2 := env.NewSession() + if err := env.SimulateUserPromptSubmit(session2.ID); err != nil { + t.Fatalf("SimulateUserPromptSubmit for session2 failed: %v", err) + } + + env.WriteFile("file3.txt", "content from session 2") + session2.CreateTranscript( + "Create file3", + []FileChange{{Path: "file3.txt", Content: "content from session 2"}}, + ) + if err := env.SimulateStop(session2.ID, session2.TranscriptPath); err != nil { + t.Fatalf("SimulateStop for session2 failed: %v", err) + } + + // Set session2 to ACTIVE (simulating mid-turn commit) + state2, err := env.GetSessionState(session2.ID) + if err != nil { + t.Fatalf("GetSessionState for session2 failed: %v", err) + } + state2.Phase = session.PhaseActive + if err := env.WriteSessionState(session2.ID, state2); err != nil { + t.Fatalf("WriteSessionState for session2 failed: %v", err) + } + + // ======================================== + // Phase 5: Commit file3.txt (unrelated to session 1's files) + // ======================================== + t.Log("Phase 5: Committing file3.txt from session 2") + + env.GitAdd("file3.txt") + env.GitCommitWithShadowHooks("Add file3 from session 2", "file3.txt") + + finalHead := env.GetHeadHash() + t.Logf("Final HEAD: %s", finalHead[:7]) + + // ======================================== + // Phase 6: Verify session 1 was NOT condensed + // ======================================== + t.Log("Phase 6: Verifying session 1 was NOT condensed into unrelated commit") + + state1After, err := env.GetSessionState(session1.ID) + if err != nil { + t.Fatalf("GetSessionState for session1 after session2 commit failed: %v", err) + } + + // StepCount should be unchanged (condensation resets it) + if state1After.StepCount != session1StepCountAfterPartial { + t.Errorf("Session 1 StepCount changed! Expected %d, got %d (was incorrectly condensed)", + session1StepCountAfterPartial, state1After.StepCount) + } + + // BaseCommit should be unchanged (ENDED sessions don't get BaseCommit updated) + if state1After.BaseCommit != session1BaseCommitAfterPartial { + t.Errorf("Session 1 BaseCommit changed! Expected %s, got %s", + session1BaseCommitAfterPartial[:7], state1After.BaseCommit[:7]) + } + + // FilesTouched should still have file2.txt (not cleared by condensation) + hasFile2After := false + for _, f := range state1After.FilesTouched { + if f == "file2.txt" { + hasFile2After = true + break + } + } + if !hasFile2After { + t.Errorf("Session 1 FilesTouched was cleared! Expected file2.txt to remain, got: %v", + state1After.FilesTouched) + } + + // Phase should still be ENDED + if state1After.Phase != session.PhaseEnded { + t.Errorf("Session 1 phase changed! Expected ENDED, got %s", state1After.Phase) + } + + // ======================================== + // Phase 7: Verify session 2 WAS condensed + // ======================================== + t.Log("Phase 7: Verifying session 2 WAS condensed") + + state2After, err := env.GetSessionState(session2.ID) + if err != nil { + t.Fatalf("GetSessionState for session2 after commit failed: %v", err) + } + + // Session 2's BaseCommit should be updated to new HEAD + if state2After.BaseCommit != finalHead { + t.Errorf("Session 2 BaseCommit should be updated to new HEAD. Expected %s, got %s", + finalHead[:7], state2After.BaseCommit[:7]) + } + + t.Log("Test completed successfully") +} + +// TestCarryForward_NotCondensedIntoMultipleUnrelatedCommits verifies that a session +// with carry-forward files is NOT condensed into MULTIPLE subsequent unrelated commits. +// +// This is a regression test for the "repeat on every commit" bug where sessions +// with carry-forward files would be re-condensed into every subsequent commit +// indefinitely, polluting checkpoints. +// +// Scenario: +// 1. Session 1 creates file1.txt and file2.txt +// 2. User commits only file1.txt (partial commit) +// 3. Session 1 gets carry-forward: FilesTouched = ["file2.txt"] +// 4. Make 3 unrelated commits (file3.txt, file4.txt, file5.txt) +// 5. Verify: Session 1 was NOT condensed into ANY of those commits +// 6. Finally commit file2.txt +// 7. Verify: Session 1 IS condensed (carry-forward consumed) +func TestCarryForward_NotCondensedIntoMultipleUnrelatedCommits(t *testing.T) { + t.Parallel() + env := NewTestEnv(t) + defer env.Cleanup() + + // ======================================== + // Setup + // ======================================== + env.InitRepo() + env.WriteFile("README.md", "# Test Repository") + env.GitAdd("README.md") + env.GitCommit("Initial commit") + env.GitCheckoutNewBranch("feature/repeat-condensation-test") + env.InitEntire(strategy.StrategyNameManualCommit) + + // ======================================== + // Phase 1: Session 1 creates multiple files + // ======================================== + t.Log("Phase 1: Session 1 creates file1.txt and file2.txt") + + session1 := env.NewSession() + if err := env.SimulateUserPromptSubmit(session1.ID); err != nil { + t.Fatalf("SimulateUserPromptSubmit for session1 failed: %v", err) + } + + env.WriteFile("file1.txt", "content from session 1 - file 1") + env.WriteFile("file2.txt", "content from session 1 - file 2") + session1.CreateTranscript( + "Create file1 and file2", + []FileChange{ + {Path: "file1.txt", Content: "content from session 1 - file 1"}, + {Path: "file2.txt", Content: "content from session 1 - file 2"}, + }, + ) + if err := env.SimulateStop(session1.ID, session1.TranscriptPath); err != nil { + t.Fatalf("SimulateStop for session1 failed: %v", err) + } + + // ======================================== + // Phase 2: Partial commit - only file1.txt + // ======================================== + t.Log("Phase 2: Committing only file1.txt (partial commit)") + + env.GitAdd("file1.txt") + env.GitCommitWithShadowHooks("Partial commit: only file1", "file1.txt") + + // Verify carry-forward + state1, err := env.GetSessionState(session1.ID) + if err != nil { + t.Fatalf("GetSessionState for session1 after partial commit failed: %v", err) + } + t.Logf("Session1 FilesTouched after partial commit: %v", state1.FilesTouched) + + // End session 1 and force hasNew=true by setting CheckpointTranscriptStart=0 + // This simulates the bug scenario where sessionHasNewContent returns true + // because the transcript has "grown" since the (reset) checkpoint start. + state1.Phase = session.PhaseEnded + state1.CheckpointTranscriptStart = 0 // Force hasNew=true for subsequent commits + if err := env.WriteSessionState(session1.ID, state1); err != nil { + t.Fatalf("WriteSessionState for session1 failed: %v", err) + } + + // Record initial state for comparison + session1InitialStepCount := state1.StepCount + + // ======================================== + // Phase 3: Make multiple unrelated commits + // ======================================== + unrelatedFiles := []string{"file3.txt", "file4.txt", "file5.txt"} + + for i, fileName := range unrelatedFiles { + t.Logf("Phase 3.%d: Making unrelated commit with %s", i+1, fileName) + + env.WriteFile(fileName, "unrelated content "+fileName) + env.GitAdd(fileName) + env.GitCommitWithShadowHooks("Add "+fileName, fileName) + + // Verify session 1 was NOT condensed after each commit + state1, err = env.GetSessionState(session1.ID) + if err != nil { + t.Fatalf("GetSessionState for session1 after commit %d failed: %v", i+1, err) + } + + if state1.StepCount != session1InitialStepCount { + t.Errorf("After commit %d (%s): Session 1 StepCount changed from %d to %d (incorrectly condensed!)", + i+1, fileName, session1InitialStepCount, state1.StepCount) + } + + // FilesTouched should still have file2.txt + hasFile2 := false + for _, f := range state1.FilesTouched { + if f == "file2.txt" { + hasFile2 = true + break + } + } + if !hasFile2 { + t.Errorf("After commit %d (%s): Session 1 FilesTouched was cleared! Expected file2.txt, got: %v", + i+1, fileName, state1.FilesTouched) + } + + t.Logf(" -> Session 1 correctly NOT condensed (StepCount=%d, FilesTouched=%v)", + state1.StepCount, state1.FilesTouched) + } + + // ======================================== + // Phase 4: Finally commit file2.txt (the carry-forward file) + // ======================================== + t.Log("Phase 4: Committing file2.txt (the carry-forward file)") + + env.GitAdd("file2.txt") + env.GitCommitWithShadowHooks("Add file2 (carry-forward)", "file2.txt") + + // ======================================== + // Phase 5: Verify session 1 WAS condensed this time + // ======================================== + t.Log("Phase 5: Verifying session 1 WAS condensed when its file was committed") + + state1, err = env.GetSessionState(session1.ID) + if err != nil { + t.Fatalf("GetSessionState for session1 after file2 commit failed: %v", err) + } + + // StepCount should be reset (condensation happened) + if state1.StepCount != 0 { + t.Errorf("Session 1 StepCount should be 0 after condensation, got %d", state1.StepCount) + } + + // FilesTouched should be empty (carry-forward consumed) + if len(state1.FilesTouched) != 0 { + t.Errorf("Session 1 FilesTouched should be empty after condensation, got: %v", state1.FilesTouched) + } + + t.Log("Test completed successfully - session correctly condensed only when its file was committed") +} + +// TestCarryForward_NewSessionCommitDoesNotCondenseOldSession verifies that when +// an old session has carry-forward files and a NEW session commits unrelated files, +// the old session is NOT condensed into the new session's commit. +// +// This tests the interaction between multiple sessions where: +// 1. Old session has carry-forward files (file2.txt) +// 2. New session creates and commits different files (file6.txt) +// 3. Old session should remain untouched +func TestCarryForward_NewSessionCommitDoesNotCondenseOldSession(t *testing.T) { + t.Parallel() + env := NewTestEnv(t) + defer env.Cleanup() + + // ======================================== + // Setup + // ======================================== + env.InitRepo() + env.WriteFile("README.md", "# Test Repository") + env.GitAdd("README.md") + env.GitCommit("Initial commit") + env.GitCheckoutNewBranch("feature/multi-session-carry-forward") + env.InitEntire(strategy.StrategyNameManualCommit) + + // ======================================== + // Phase 1: Session 1 creates files, partial commit, ends with carry-forward + // ======================================== + t.Log("Phase 1: Session 1 creates file1.txt and file2.txt") + + session1 := env.NewSession() + if err := env.SimulateUserPromptSubmit(session1.ID); err != nil { + t.Fatalf("SimulateUserPromptSubmit for session1 failed: %v", err) + } + + env.WriteFile("file1.txt", "content from session 1 - file 1") + env.WriteFile("file2.txt", "content from session 1 - file 2") + session1.CreateTranscript( + "Create file1 and file2", + []FileChange{ + {Path: "file1.txt", Content: "content from session 1 - file 1"}, + {Path: "file2.txt", Content: "content from session 1 - file 2"}, + }, + ) + if err := env.SimulateStop(session1.ID, session1.TranscriptPath); err != nil { + t.Fatalf("SimulateStop for session1 failed: %v", err) + } + + // Partial commit - only file1.txt + t.Log("Phase 1b: Partial commit - only file1.txt") + env.GitAdd("file1.txt") + env.GitCommitWithShadowHooks("Partial commit: only file1", "file1.txt") + + // End session 1 + state1, err := env.GetSessionState(session1.ID) + if err != nil { + t.Fatalf("GetSessionState for session1 failed: %v", err) + } + state1.Phase = session.PhaseEnded + if err := env.WriteSessionState(session1.ID, state1); err != nil { + t.Fatalf("WriteSessionState for session1 failed: %v", err) + } + + // Verify carry-forward + state1, err = env.GetSessionState(session1.ID) + if err != nil { + t.Fatalf("GetSessionState for session1 failed: %v", err) + } + t.Logf("Session1 (ENDED) FilesTouched: %v", state1.FilesTouched) + + session1StepCount := state1.StepCount + + // ======================================== + // Phase 2: Make some unrelated commits (simulating time passing) + // ======================================== + t.Log("Phase 2: Making unrelated commits") + + for _, fileName := range []string{"file3.txt", "file4.txt"} { + env.WriteFile(fileName, "unrelated content") + env.GitAdd(fileName) + env.GitCommitWithShadowHooks("Add "+fileName, fileName) + } + + // ======================================== + // Phase 3: NEW session 2 starts and creates file6.txt + // ======================================== + t.Log("Phase 3: Session 2 starts and creates file6.txt") + + session2 := env.NewSession() + if err := env.SimulateUserPromptSubmit(session2.ID); err != nil { + t.Fatalf("SimulateUserPromptSubmit for session2 failed: %v", err) + } + + env.WriteFile("file6.txt", "content from session 2") + session2.CreateTranscript( + "Create file6", + []FileChange{{Path: "file6.txt", Content: "content from session 2"}}, + ) + if err := env.SimulateStop(session2.ID, session2.TranscriptPath); err != nil { + t.Fatalf("SimulateStop for session2 failed: %v", err) + } + + // Set session2 to ACTIVE + state2, err := env.GetSessionState(session2.ID) + if err != nil { + t.Fatalf("GetSessionState for session2 failed: %v", err) + } + state2.Phase = session.PhaseActive + if err := env.WriteSessionState(session2.ID, state2); err != nil { + t.Fatalf("WriteSessionState for session2 failed: %v", err) + } + + // ======================================== + // Phase 4: Commit file6.txt (session 2's file) + // ======================================== + t.Log("Phase 4: Committing file6.txt from session 2") + + env.GitAdd("file6.txt") + env.GitCommitWithShadowHooks("Add file6 from session 2", "file6.txt") + + finalHead := env.GetHeadHash() + + // ======================================== + // Phase 5: Verify session 1 was NOT condensed + // ======================================== + t.Log("Phase 5: Verifying session 1 (with carry-forward) was NOT condensed") + + state1After, err := env.GetSessionState(session1.ID) + if err != nil { + t.Fatalf("GetSessionState for session1 after session2 commit failed: %v", err) + } + + // StepCount should be unchanged + if state1After.StepCount != session1StepCount { + t.Errorf("Session 1 StepCount changed! Expected %d, got %d (incorrectly condensed into session 2's commit)", + session1StepCount, state1After.StepCount) + } + + // FilesTouched should still have file2.txt + hasFile2 := false + for _, f := range state1After.FilesTouched { + if f == "file2.txt" { + hasFile2 = true + break + } + } + if !hasFile2 { + t.Errorf("Session 1 FilesTouched was cleared! Expected file2.txt, got: %v", state1After.FilesTouched) + } + + t.Logf("Session 1 correctly preserved: StepCount=%d, FilesTouched=%v", state1After.StepCount, state1After.FilesTouched) + + // ======================================== + // Phase 6: Verify session 2 WAS condensed + // ======================================== + t.Log("Phase 6: Verifying session 2 WAS condensed") + + state2After, err := env.GetSessionState(session2.ID) + if err != nil { + t.Fatalf("GetSessionState for session2 after commit failed: %v", err) + } + + if state2After.BaseCommit != finalHead { + t.Errorf("Session 2 BaseCommit should be updated. Expected %s, got %s", + finalHead[:7], state2After.BaseCommit[:7]) + } + + // ======================================== + // Phase 7: Finally commit file2.txt (session 1's carry-forward file) + // ======================================== + t.Log("Phase 7: Committing file2.txt (session 1's carry-forward file)") + + env.GitAdd("file2.txt") + env.GitCommitWithShadowHooks("Add file2 (session 1 carry-forward)", "file2.txt") + + // ======================================== + // Phase 8: Verify session 1 WAS condensed this time + // ======================================== + t.Log("Phase 8: Verifying session 1 WAS condensed when its carry-forward file was committed") + + state1Final, err := env.GetSessionState(session1.ID) + if err != nil { + t.Fatalf("GetSessionState for session1 after file2 commit failed: %v", err) + } + + // StepCount should be reset to 0 (condensation happened) + if state1Final.StepCount != 0 { + t.Errorf("Session 1 StepCount should be 0 after condensation, got %d", state1Final.StepCount) + } + + // FilesTouched should be empty (carry-forward consumed) + if len(state1Final.FilesTouched) != 0 { + t.Errorf("Session 1 FilesTouched should be empty after condensation, got: %v", state1Final.FilesTouched) + } + + t.Log("Test completed successfully:") + t.Log(" - Session 1 NOT condensed into session 2's commit (file6.txt)") + t.Log(" - Session 1 WAS condensed when its own file (file2.txt) was committed") +} + +// TestStaleActiveSession_NotCondensedIntoUnrelatedCommit verifies that a stale +// ACTIVE session (agent resumed then killed) is NOT condensed into an unrelated +// commit from a different session. +// +// This is a regression test for the bug where ACTIVE sessions would always be +// condensed (hasNew=true) without checking file overlap. +// +// Scenario: +// 1. Session 1 creates file1.txt, Stop hook fires (checkpoint saved) +// 2. Session 1 is resumed (back to ACTIVE), does more work +// 3. Agent is killed (stays ACTIVE, new work not committed) +// 4. New session 2 creates file2.txt (unrelated) +// 5. Session 2 commits file2.txt +// 6. Verify: Session 1 was NOT condensed (its files weren't in the commit) +func TestStaleActiveSession_NotCondensedIntoUnrelatedCommit(t *testing.T) { + t.Parallel() + env := NewTestEnv(t) + defer env.Cleanup() + + // ======================================== + // Setup + // ======================================== + env.InitRepo() + env.WriteFile("README.md", "# Test Repository") + env.GitAdd("README.md") + env.GitCommit("Initial commit") + env.GitCheckoutNewBranch("feature/stale-active-test") + env.InitEntire(strategy.StrategyNameManualCommit) + + // ======================================== + // Phase 1: Session 1 creates file, Stop hook fires (creates checkpoint) + // ======================================== + t.Log("Phase 1: Session 1 creates file1.txt, Stop hook fires") + + session1 := env.NewSession() + if err := env.SimulateUserPromptSubmit(session1.ID); err != nil { + t.Fatalf("SimulateUserPromptSubmit for session1 failed: %v", err) + } + + env.WriteFile("file1.txt", "content from session 1") + session1.CreateTranscript( + "Create file1", + []FileChange{{Path: "file1.txt", Content: "content from session 1"}}, + ) + if err := env.SimulateStop(session1.ID, session1.TranscriptPath); err != nil { + t.Fatalf("SimulateStop for session1 failed: %v", err) + } + + // Session 1 is now IDLE with a checkpoint + state1, err := env.GetSessionState(session1.ID) + if err != nil { + t.Fatalf("GetSessionState for session1 failed: %v", err) + } + t.Logf("Session1 after Stop: Phase=%s, StepCount=%d, CheckpointTranscriptStart=%d", + state1.Phase, state1.StepCount, state1.CheckpointTranscriptStart) + + // ======================================== + // Phase 2: Simulate agent resume + crash (ACTIVE with hasNew=true) + // Set ACTIVE and reset CheckpointTranscriptStart to simulate: + // - Agent was resumed (ACTIVE) + // - Did more work (transcript grew, but not saved to checkpoint yet) + // - Then crashed (stuck ACTIVE with hasNew=true) + // ======================================== + t.Log("Phase 2: Simulating agent resume then crash (ACTIVE with hasNew=true)") + + state1.Phase = session.PhaseActive + state1.CheckpointTranscriptStart = 0 // Force hasNew=true (transcript > 0) + if err := env.WriteSessionState(session1.ID, state1); err != nil { + t.Fatalf("WriteSessionState for session1 failed: %v", err) + } + + // Record state for comparison + session1StepCount := state1.StepCount + t.Logf("Session1 (stale ACTIVE) StepCount: %d, CheckpointTranscriptStart: %d (forced to 0 for hasNew=true)", + session1StepCount, state1.CheckpointTranscriptStart) + + // ======================================== + // Phase 2b: Make an unrelated commit to move HEAD forward + // This ensures Session 1 and Session 2 have DIFFERENT shadow branches + // (different BaseCommit), isolating their condensation behavior. + // ======================================== + t.Log("Phase 2b: Making unrelated commit to separate shadow branches") + env.WriteFile("unrelated.txt", "unrelated content") + env.GitAdd("unrelated.txt") + env.GitCommit("Unrelated commit to move HEAD") + + // ======================================== + // Phase 3: New session 2 creates unrelated file + // ======================================== + t.Log("Phase 3: Session 2 creates file2.txt (unrelated to session 1)") + + session2 := env.NewSession() + if err := env.SimulateUserPromptSubmit(session2.ID); err != nil { + t.Fatalf("SimulateUserPromptSubmit for session2 failed: %v", err) + } + + env.WriteFile("file2.txt", "content from session 2") + session2.CreateTranscript( + "Create file2", + []FileChange{{Path: "file2.txt", Content: "content from session 2"}}, + ) + if err := env.SimulateStop(session2.ID, session2.TranscriptPath); err != nil { + t.Fatalf("SimulateStop for session2 failed: %v", err) + } + + // Set session2 to ACTIVE + state2, err := env.GetSessionState(session2.ID) + if err != nil { + t.Fatalf("GetSessionState for session2 failed: %v", err) + } + state2.Phase = session.PhaseActive + if err := env.WriteSessionState(session2.ID, state2); err != nil { + t.Fatalf("WriteSessionState for session2 failed: %v", err) + } + + // ======================================== + // Phase 4: Commit file2.txt (unrelated to session 1) + // ======================================== + t.Log("Phase 4: Committing file2.txt from session 2") + + env.GitAdd("file2.txt") + env.GitCommitWithShadowHooks("Add file2 from session 2", "file2.txt") + + finalHead := env.GetHeadHash() + t.Logf("Final HEAD: %s", finalHead[:7]) + + // ======================================== + // Phase 5: Verify stale session 1 was NOT condensed + // ======================================== + t.Log("Phase 5: Verifying stale session 1 was NOT condensed") + + state1After, err := env.GetSessionState(session1.ID) + if err != nil { + t.Fatalf("GetSessionState for session1 after session2 commit failed: %v", err) + } + + // StepCount should be unchanged (condensation resets it) + if state1After.StepCount != session1StepCount { + t.Errorf("Stale session 1 StepCount changed! Expected %d, got %d (was incorrectly condensed)", + session1StepCount, state1After.StepCount) + } + + // Note: BaseCommit WILL be updated for ACTIVE sessions (even stale ones) because + // updateBaseCommitIfChanged only skips IDLE/ENDED sessions. This is fine - the + // key fix is the overlap check that prevents condensation, not BaseCommit updates. + // A stale ACTIVE session that gets resumed later needs to know the current HEAD. + + // FilesTouched should still have file1.txt (not cleared by condensation) + hasFile1 := false + for _, f := range state1After.FilesTouched { + if f == "file1.txt" { + hasFile1 = true + break + } + } + if !hasFile1 { + t.Errorf("Stale session 1 FilesTouched was cleared! Expected file1.txt, got: %v", + state1After.FilesTouched) + } + + // ======================================== + // Phase 6: Verify session 2 WAS condensed + // ======================================== + t.Log("Phase 6: Verifying session 2 WAS condensed") + + state2After, err := env.GetSessionState(session2.ID) + if err != nil { + t.Fatalf("GetSessionState for session2 after commit failed: %v", err) + } + + if state2After.BaseCommit != finalHead { + t.Errorf("Session 2 BaseCommit should be updated to new HEAD. Expected %s, got %s", + finalHead[:7], state2After.BaseCommit[:7]) + } + + t.Log("Test completed successfully") +} + +// TestIdleSessionEmptyFilesTouched_NotCondensedIntoUnrelatedCommit verifies that +// an IDLE session with empty FilesTouched (e.g., conversation-only session) is NOT +// condensed into an unrelated commit. +// +// This is a regression test for the fail-open bug where shouldCondenseWithOverlapCheck +// would return true when filesTouchedBefore was empty, causing conversation-only +// sessions to be condensed into every commit. +// +// Scenario: +// 1. Session 1 has a conversation but doesn't modify any files +// 2. New session 2 creates file1.txt +// 3. Session 2 commits file1.txt +// 4. Verify: Session 1 was NOT condensed +func TestIdleSessionEmptyFilesTouched_NotCondensedIntoUnrelatedCommit(t *testing.T) { + t.Parallel() + env := NewTestEnv(t) + defer env.Cleanup() + + // ======================================== + // Setup + // ======================================== + env.InitRepo() + env.WriteFile("README.md", "# Test Repository") + env.GitAdd("README.md") + env.GitCommit("Initial commit") + env.GitCheckoutNewBranch("feature/empty-files-test") + env.InitEntire(strategy.StrategyNameManualCommit) + + // ======================================== + // Phase 1: Session 1 - conversation only, no file changes + // ======================================== + t.Log("Phase 1: Session 1 has conversation but no file changes") + + session1 := env.NewSession() + if err := env.SimulateUserPromptSubmit(session1.ID); err != nil { + t.Fatalf("SimulateUserPromptSubmit for session1 failed: %v", err) + } + + // Create transcript with NO file changes + session1.CreateTranscript( + "What is the meaning of life?", + []FileChange{}, // No files touched + ) + if err := env.SimulateStop(session1.ID, session1.TranscriptPath); err != nil { + t.Fatalf("SimulateStop for session1 failed: %v", err) + } + + // Verify session1 is IDLE with empty FilesTouched + state1, err := env.GetSessionState(session1.ID) + if err != nil { + t.Fatalf("GetSessionState for session1 failed: %v", err) + } + if state1.Phase != session.PhaseIdle { + t.Fatalf("Expected session1 to be IDLE, got %s", state1.Phase) + } + if len(state1.FilesTouched) != 0 { + t.Fatalf("Expected session1 FilesTouched to be empty, got: %v", state1.FilesTouched) + } + + session1StepCount := state1.StepCount + session1BaseCommit := state1.BaseCommit + t.Logf("Session1 (conversation-only) BaseCommit: %s, StepCount: %d", + session1BaseCommit[:7], session1StepCount) + + // ======================================== + // Phase 2: Session 2 creates a file + // ======================================== + t.Log("Phase 2: Session 2 creates file1.txt") + + session2 := env.NewSession() + if err := env.SimulateUserPromptSubmit(session2.ID); err != nil { + t.Fatalf("SimulateUserPromptSubmit for session2 failed: %v", err) + } + + env.WriteFile("file1.txt", "content from session 2") + session2.CreateTranscript( + "Create file1", + []FileChange{{Path: "file1.txt", Content: "content from session 2"}}, + ) + if err := env.SimulateStop(session2.ID, session2.TranscriptPath); err != nil { + t.Fatalf("SimulateStop for session2 failed: %v", err) + } + + // Set session2 to ACTIVE + state2, err := env.GetSessionState(session2.ID) + if err != nil { + t.Fatalf("GetSessionState for session2 failed: %v", err) + } + state2.Phase = session.PhaseActive + if err := env.WriteSessionState(session2.ID, state2); err != nil { + t.Fatalf("WriteSessionState for session2 failed: %v", err) + } + + // ======================================== + // Phase 3: Commit file1.txt + // ======================================== + t.Log("Phase 3: Committing file1.txt from session 2") + + env.GitAdd("file1.txt") + env.GitCommitWithShadowHooks("Add file1 from session 2", "file1.txt") + + finalHead := env.GetHeadHash() + + // ======================================== + // Phase 4: Verify conversation-only session 1 was NOT condensed + // ======================================== + t.Log("Phase 4: Verifying conversation-only session 1 was NOT condensed") + + state1After, err := env.GetSessionState(session1.ID) + if err != nil { + t.Fatalf("GetSessionState for session1 after session2 commit failed: %v", err) + } + + // StepCount should be unchanged + if state1After.StepCount != session1StepCount { + t.Errorf("Conversation-only session 1 StepCount changed! Expected %d, got %d", + session1StepCount, state1After.StepCount) + } + + // BaseCommit should be unchanged + if state1After.BaseCommit != session1BaseCommit { + t.Errorf("Conversation-only session 1 BaseCommit changed! Expected %s, got %s", + session1BaseCommit[:7], state1After.BaseCommit[:7]) + } + + // FilesTouched should still be empty + if len(state1After.FilesTouched) != 0 { + t.Errorf("Conversation-only session 1 FilesTouched should remain empty, got: %v", + state1After.FilesTouched) + } + + // ======================================== + // Phase 6: Verify session 2 WAS condensed + // ======================================== + t.Log("Phase 6: Verifying session 2 WAS condensed") + + state2After, err := env.GetSessionState(session2.ID) + if err != nil { + t.Fatalf("GetSessionState for session2 after commit failed: %v", err) + } + + if state2After.BaseCommit != finalHead { + t.Errorf("Session 2 BaseCommit should be updated to new HEAD. Expected %s, got %s", + finalHead[:7], state2After.BaseCommit[:7]) + } + + t.Log("Test completed successfully") +} diff --git a/cmd/entire/cli/integration_test/mid_session_commit_test.go b/cmd/entire/cli/integration_test/mid_session_commit_test.go index 65437952e..c8147e8d1 100644 --- a/cmd/entire/cli/integration_test/mid_session_commit_test.go +++ b/cmd/entire/cli/integration_test/mid_session_commit_test.go @@ -210,3 +210,78 @@ func TestShadowStrategy_AgentCommit_AlwaysGetsTrailer(t *testing.T) { t.Logf("Agent commit correctly got checkpoint trailer: %s", checkpointID) } } + +// TestShadowStrategy_MidSessionCommit_FilesTouchedFallback tests that when +// FilesTouched is empty in session state (mid-session commit before SaveStep), +// the fallback to committedFiles works correctly and the checkpoint metadata +// contains the files that were actually committed. +// +// This is scenario 1 from the fix: when FilesTouched was originally empty, +// fallback should assign committedFiles to files_touched. +func TestShadowStrategy_MidSessionCommit_FilesTouchedFallback(t *testing.T) { + t.Parallel() + + env := NewFeatureBranchEnv(t, strategy.StrategyNameManualCommit) + + session := env.NewSession() + + // Simulate user prompt with transcript path (initializes session with empty FilesTouched) + input := map[string]string{ + "session_id": session.ID, + "transcript_path": session.TranscriptPath, + } + inputJSON, _ := json.Marshal(input) + cmd := exec.Command(getTestBinary(), "hooks", "claude-code", "user-prompt-submit") + cmd.Dir = env.RepoDir + cmd.Stdin = bytes.NewReader(inputJSON) + cmd.Env = append(os.Environ(), + "ENTIRE_TEST_CLAUDE_PROJECT_DIR="+env.ClaudeProjectDir, + ) + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("user-prompt-submit failed: %v\nOutput: %s", err, output) + } + + // Verify session state has empty FilesTouched (no SaveStep has been called) + state, err := env.GetSessionState(session.ID) + if err != nil { + t.Fatalf("Failed to get session state: %v", err) + } + if state == nil { + t.Fatal("Session state is nil") + } + if len(state.FilesTouched) != 0 { + t.Errorf("Session state FilesTouched should be empty before SaveStep, got: %v", state.FilesTouched) + } + + // Create a file as if Claude wrote it + env.WriteFile("mid_session_file.txt", "content from Claude mid-session") + + // Create transcript showing Claude created the file (NO Stop called, NO SaveStep called) + session.CreateTranscript("Create a file for testing fallback", []FileChange{ + {Path: "mid_session_file.txt", Content: "content from Claude mid-session"}, + }) + + // Commit mid-session - FilesTouched in session state is still empty + // The fallback should assign committedFiles to files_touched in the checkpoint metadata + env.GitCommitWithShadowHooks("Mid-session commit testing fallback", "mid_session_file.txt") + + // Get the checkpoint ID from the commit + commitHash := env.GetHeadHash() + checkpointID := env.GetCheckpointIDFromCommitMessage(commitHash) + if checkpointID == "" { + t.Fatal("Mid-session commit should have Entire-Checkpoint trailer") + } + t.Logf("Mid-session commit has checkpoint ID: %s", checkpointID) + + // CRITICAL: Validate that the checkpoint metadata has the correct files_touched + // This verifies the fallback logic: when FilesTouched was empty, it should + // have been populated with the committed files. + env.ValidateCheckpoint(CheckpointValidation{ + CheckpointID: checkpointID, + SessionID: session.ID, + Strategy: strategy.StrategyNameManualCommit, + FilesTouched: []string{"mid_session_file.txt"}, + }) + + t.Log("FilesTouched fallback worked correctly: checkpoint metadata contains the committed file") +} diff --git a/cmd/entire/cli/strategy/manual_commit_condensation.go b/cmd/entire/cli/strategy/manual_commit_condensation.go index fc24205d7..1e6beb00c 100644 --- a/cmd/entire/cli/strategy/manual_commit_condensation.go +++ b/cmd/entire/cli/strategy/manual_commit_condensation.go @@ -146,7 +146,12 @@ func (s *ManualCommitStrategy) CondenseSession(repo *git.Repository, checkpointI // committed in this specific commit. This ensures each checkpoint represents // exactly the files in that commit, not all files mentioned in the transcript. if len(committedFiles) > 0 { - if len(sessionData.FilesTouched) > 0 { + // Track if we had files before filtering to distinguish between: + // - Session had files but none were committed (don't fallback) + // - Session had no files to begin with (mid-session commit, fallback OK) + hadFilesBeforeFiltering := len(sessionData.FilesTouched) > 0 + + if hadFilesBeforeFiltering { // Filter to intersection of transcript-extracted files and committed files filtered := make([]string, 0, len(sessionData.FilesTouched)) for _, f := range sessionData.FilesTouched { @@ -157,10 +162,12 @@ func (s *ManualCommitStrategy) CondenseSession(repo *git.Repository, checkpointI sessionData.FilesTouched = filtered } - // If extraction failed or returned empty, use committedFiles as fallback. - // This handles mid-session commits where transcript parsing may not find files - // but we know what was committed. - if len(sessionData.FilesTouched) == 0 { + // Only use committedFiles as fallback for genuine mid-session commits where + // no files were tracked yet (extraction returned empty). Do NOT fallback when + // the session had files that simply didn't overlap with the commit - that + // indicates an unrelated session that shouldn't have its files_touched + // overwritten with unrelated committed files. + if len(sessionData.FilesTouched) == 0 && !hadFilesBeforeFiltering { sessionData.FilesTouched = make([]string, 0, len(committedFiles)) for f := range committedFiles { sessionData.FilesTouched = append(sessionData.FilesTouched, f) diff --git a/cmd/entire/cli/strategy/manual_commit_test.go b/cmd/entire/cli/strategy/manual_commit_test.go index 3f9c1d8ea..ed343c3f6 100644 --- a/cmd/entire/cli/strategy/manual_commit_test.go +++ b/cmd/entire/cli/strategy/manual_commit_test.go @@ -3352,3 +3352,233 @@ func TestCondenseSession_GeminiMultiCheckpoint(t *testing.T) { t.Error("Prompts should contain second prompt") } } + +// TestCondenseSession_FilesTouchedFallback_EmptyState verifies that when state.FilesTouched +// is empty (mid-session commit before SaveStep), the fallback to committedFiles works. +// This is the legitimate use case for the fallback. +func TestCondenseSession_FilesTouchedFallback_EmptyState(t *testing.T) { + dir := t.TempDir() + repo, err := git.PlainInit(dir, false) + if err != nil { + t.Fatalf("failed to init repo: %v", err) + } + + worktree, err := repo.Worktree() + if err != nil { + t.Fatalf("failed to get worktree: %v", err) + } + + // Create initial commit + initialHash, err := worktree.Commit("Initial commit", &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@test.com", When: time.Now()}, + AllowEmptyCommits: true, + }) + if err != nil { + t.Fatalf("failed to create initial commit: %v", err) + } + + // Create a file and commit it (simulating agent mid-turn commit) + agentFile := filepath.Join(dir, "agent.go") + if err := os.WriteFile(agentFile, []byte("package main\n"), 0o644); err != nil { + t.Fatalf("failed to write file: %v", err) + } + if _, err := worktree.Add("agent.go"); err != nil { + t.Fatalf("failed to stage file: %v", err) + } + if _, err = worktree.Commit("Add agent.go", &git.CommitOptions{ + Author: &object.Signature{Name: "Agent", Email: "agent@test.com", When: time.Now()}, + }); err != nil { + t.Fatalf("failed to commit: %v", err) + } + + t.Chdir(dir) + + // Create live transcript (required when no shadow branch) + transcriptDir := filepath.Join(dir, ".claude", "projects", "test") + if err := os.MkdirAll(transcriptDir, 0o755); err != nil { + t.Fatalf("failed to create transcript dir: %v", err) + } + transcriptFile := filepath.Join(transcriptDir, "session.jsonl") + if err := os.WriteFile(transcriptFile, []byte(`{"type":"human","message":{"content":"create agent.go"}} +{"type":"assistant","message":{"content":"Done"}} +`), 0o644); err != nil { + t.Fatalf("failed to write transcript: %v", err) + } + + // Session state with EMPTY FilesTouched (mid-session commit scenario) + state := &SessionState{ + SessionID: "test-empty-files", + BaseCommit: initialHash.String(), + FilesTouched: []string{}, // Empty - no SaveStep called yet + TranscriptPath: transcriptFile, + AgentType: "Claude Code", + } + + s := &ManualCommitStrategy{} + checkpointID := id.MustCheckpointID("fa11bac00001") + + // Condense with committedFiles - should fallback since FilesTouched is empty + committedFiles := map[string]struct{}{"agent.go": {}} + result, err := s.CondenseSession(repo, checkpointID, state, committedFiles) + if err != nil { + t.Fatalf("CondenseSession() error = %v", err) + } + + // Read metadata and verify files_touched contains the committed file (fallback worked) + sessionsRef, err := repo.Reference(plumbing.NewBranchReferenceName(paths.MetadataBranchName), true) + if err != nil { + t.Fatalf("failed to get sessions branch: %v", err) + } + sessionsCommit, err := repo.CommitObject(sessionsRef.Hash()) + if err != nil { + t.Fatalf("failed to get sessions commit: %v", err) + } + tree, err := sessionsCommit.Tree() + if err != nil { + t.Fatalf("failed to get tree: %v", err) + } + + metadataPath := checkpointID.Path() + "/0/" + paths.MetadataFileName + metadataFile, err := tree.File(metadataPath) + if err != nil { + t.Fatalf("failed to find metadata: %v", err) + } + content, err := metadataFile.Contents() + if err != nil { + t.Fatalf("failed to read metadata: %v", err) + } + + var metadata struct { + FilesTouched []string `json:"files_touched"` + } + if err := json.Unmarshal([]byte(content), &metadata); err != nil { + t.Fatalf("failed to parse metadata: %v", err) + } + + // Verify fallback worked - files_touched should contain agent.go + if len(metadata.FilesTouched) != 1 || metadata.FilesTouched[0] != "agent.go" { + t.Errorf("files_touched = %v, want [agent.go] (fallback should apply when FilesTouched is empty)", + metadata.FilesTouched) + } + + t.Logf("Fallback worked: files_touched = %v, result = %+v", metadata.FilesTouched, result) +} + +// TestCondenseSession_FilesTouchedNoFallback_NoOverlap verifies that when state.FilesTouched +// has files but none overlap with committedFiles, we do NOT fallback to committedFiles. +// This prevents the bug where unrelated sessions get incorrect files_touched. +func TestCondenseSession_FilesTouchedNoFallback_NoOverlap(t *testing.T) { + dir := t.TempDir() + repo, err := git.PlainInit(dir, false) + if err != nil { + t.Fatalf("failed to init repo: %v", err) + } + + worktree, err := repo.Worktree() + if err != nil { + t.Fatalf("failed to get worktree: %v", err) + } + + // Create initial commit + initialHash, err := worktree.Commit("Initial commit", &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@test.com", When: time.Now()}, + AllowEmptyCommits: true, + }) + if err != nil { + t.Fatalf("failed to create initial commit: %v", err) + } + + // Create files for both the session's work and the committed file + sessionFile := filepath.Join(dir, "session_file.go") + if err := os.WriteFile(sessionFile, []byte("package session\n"), 0o644); err != nil { + t.Fatalf("failed to write session file: %v", err) + } + committedFile := filepath.Join(dir, "other_file.go") + if err := os.WriteFile(committedFile, []byte("package other\n"), 0o644); err != nil { + t.Fatalf("failed to write committed file: %v", err) + } + + // Only commit the "other" file (not the session's file) + if _, err := worktree.Add("other_file.go"); err != nil { + t.Fatalf("failed to stage file: %v", err) + } + if _, err = worktree.Commit("Add other_file.go", &git.CommitOptions{ + Author: &object.Signature{Name: "Human", Email: "human@test.com", When: time.Now()}, + }); err != nil { + t.Fatalf("failed to commit: %v", err) + } + + t.Chdir(dir) + + // Create live transcript + transcriptDir := filepath.Join(dir, ".claude", "projects", "test") + if err := os.MkdirAll(transcriptDir, 0o755); err != nil { + t.Fatalf("failed to create transcript dir: %v", err) + } + transcriptFile := filepath.Join(transcriptDir, "session.jsonl") + if err := os.WriteFile(transcriptFile, []byte(`{"type":"human","message":{"content":"work on session_file.go"}} +{"type":"assistant","message":{"content":"Done"}} +`), 0o644); err != nil { + t.Fatalf("failed to write transcript: %v", err) + } + + // Session state with FilesTouched that does NOT overlap with committedFiles + state := &SessionState{ + SessionID: "test-no-overlap", + BaseCommit: initialHash.String(), + FilesTouched: []string{"session_file.go"}, // Does NOT overlap with other_file.go + TranscriptPath: transcriptFile, + AgentType: "Claude Code", + } + + s := &ManualCommitStrategy{} + checkpointID := id.MustCheckpointID("00001a000001") + + // Condense with committedFiles that don't overlap + committedFiles := map[string]struct{}{"other_file.go": {}} + result, err := s.CondenseSession(repo, checkpointID, state, committedFiles) + if err != nil { + t.Fatalf("CondenseSession() error = %v", err) + } + + // Read metadata and verify files_touched is EMPTY (no fallback applied) + sessionsRef, err := repo.Reference(plumbing.NewBranchReferenceName(paths.MetadataBranchName), true) + if err != nil { + t.Fatalf("failed to get sessions branch: %v", err) + } + sessionsCommit, err := repo.CommitObject(sessionsRef.Hash()) + if err != nil { + t.Fatalf("failed to get sessions commit: %v", err) + } + tree, err := sessionsCommit.Tree() + if err != nil { + t.Fatalf("failed to get tree: %v", err) + } + + metadataPath := checkpointID.Path() + "/0/" + paths.MetadataFileName + metadataFile, err := tree.File(metadataPath) + if err != nil { + t.Fatalf("failed to find metadata: %v", err) + } + content, err := metadataFile.Contents() + if err != nil { + t.Fatalf("failed to read metadata: %v", err) + } + + var metadata struct { + FilesTouched []string `json:"files_touched"` + } + if err := json.Unmarshal([]byte(content), &metadata); err != nil { + t.Fatalf("failed to parse metadata: %v", err) + } + + // Verify NO fallback - files_touched should be EMPTY, NOT contain other_file.go + // This is the key fix: session had files (session_file.go) but none overlapped, + // so we should NOT fallback to committedFiles (other_file.go) + if len(metadata.FilesTouched) != 0 { + t.Errorf("files_touched = %v, want [] (should NOT fallback when session had files but no overlap)", + metadata.FilesTouched) + } + + t.Logf("No fallback applied: files_touched = %v (correctly empty), result = %+v", metadata.FilesTouched, result) +} From 5f515e0932a22a2b2cd1f4bc2f24a8b9d07a2f70 Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Thu, 19 Feb 2026 11:32:24 +0100 Subject: [PATCH 6/7] cleanup Entire-Checkpoint: 4e61e28fd507 --- .../carry_forward_overlap_test.go | 697 +----------------- 1 file changed, 12 insertions(+), 685 deletions(-) diff --git a/cmd/entire/cli/integration_test/carry_forward_overlap_test.go b/cmd/entire/cli/integration_test/carry_forward_overlap_test.go index 858650fb5..2a48910ef 100644 --- a/cmd/entire/cli/integration_test/carry_forward_overlap_test.go +++ b/cmd/entire/cli/integration_test/carry_forward_overlap_test.go @@ -9,372 +9,26 @@ import ( "github.com/entireio/cli/cmd/entire/cli/strategy" ) -// TestCarryForward_NotCondensedIntoUnrelatedCommit verifies that a session with -// carry-forward files is NOT condensed into a commit that doesn't touch those files. +// TestCarryForward_NewSessionCommitDoesNotCondenseOldSession verifies that when +// an old session has carry-forward files and a NEW session commits unrelated files, +// the old session is NOT condensed into the new session's commit. // // This is a regression test for the bug where sessions with carry-forward files -// would be re-condensed into every subsequent commit indefinitely. The root cause -// was that HandleCondense and HandleCondenseIfFilesTouched only checked hasNew -// (shadow branch has content) but didn't verify that the committed files actually -// overlapped with the session's FilesTouched. +// would be re-condensed into every subsequent commit indefinitely. // -// Scenario: -// 1. Session 1 creates file1.txt and file2.txt -// 2. User commits ONLY file1.txt (partial commit) -// 3. Session 1 gets carry-forward: FilesTouched = ["file2.txt"] -// 4. Session 1 ends -// 5. New session 2 creates file3.txt (unrelated to session 1) -// 6. Session 2 commits file3.txt -// 7. Verify: Session 1 was NOT condensed (FilesTouched preserved, StepCount unchanged) -func TestCarryForward_NotCondensedIntoUnrelatedCommit(t *testing.T) { - t.Parallel() - env := NewTestEnv(t) - defer env.Cleanup() - - // ======================================== - // Setup - // ======================================== - env.InitRepo() - env.WriteFile("README.md", "# Test Repository") - env.GitAdd("README.md") - env.GitCommit("Initial commit") - env.GitCheckoutNewBranch("feature/carry-forward-test") - env.InitEntire(strategy.StrategyNameManualCommit) - - // ======================================== - // Phase 1: Session 1 creates multiple files - // ======================================== - t.Log("Phase 1: Session 1 creates file1.txt and file2.txt") - - session1 := env.NewSession() - if err := env.SimulateUserPromptSubmit(session1.ID); err != nil { - t.Fatalf("SimulateUserPromptSubmit for session1 failed: %v", err) - } - - // Create two files - env.WriteFile("file1.txt", "content from session 1 - file 1") - env.WriteFile("file2.txt", "content from session 1 - file 2") - session1.CreateTranscript( - "Create file1 and file2", - []FileChange{ - {Path: "file1.txt", Content: "content from session 1 - file 1"}, - {Path: "file2.txt", Content: "content from session 1 - file 2"}, - }, - ) - if err := env.SimulateStop(session1.ID, session1.TranscriptPath); err != nil { - t.Fatalf("SimulateStop for session1 failed: %v", err) - } - - // Verify session1 is IDLE with both files in FilesTouched - state1, err := env.GetSessionState(session1.ID) - if err != nil { - t.Fatalf("GetSessionState for session1 failed: %v", err) - } - if state1.Phase != session.PhaseIdle { - t.Fatalf("Expected session1 to be IDLE, got %s", state1.Phase) - } - t.Logf("Session1 FilesTouched before partial commit: %v", state1.FilesTouched) - - // ======================================== - // Phase 2: Partial commit - only file1.txt - // ======================================== - t.Log("Phase 2: Committing only file1.txt (partial commit)") - - env.GitAdd("file1.txt") - // Note: file2.txt is NOT staged - env.GitCommitWithShadowHooks("Partial commit: only file1", "file1.txt") - - // Verify carry-forward: file2.txt should remain in FilesTouched - state1, err = env.GetSessionState(session1.ID) - if err != nil { - t.Fatalf("GetSessionState for session1 after partial commit failed: %v", err) - } - t.Logf("Session1 FilesTouched after partial commit: %v", state1.FilesTouched) - - // file2.txt should be carried forward (not committed yet) - hasFile2 := false - for _, f := range state1.FilesTouched { - if f == "file2.txt" { - hasFile2 = true - break - } - } - if !hasFile2 { - t.Fatalf("Expected file2.txt to be carried forward in FilesTouched, got: %v", state1.FilesTouched) - } - - // Record state for later comparison - session1StepCountAfterPartial := state1.StepCount - session1BaseCommitAfterPartial := state1.BaseCommit - - // ======================================== - // Phase 3: End session 1 (simulating user closes agent) - // ======================================== - t.Log("Phase 3: Ending session 1") - - state1.Phase = session.PhaseEnded - if err := env.WriteSessionState(session1.ID, state1); err != nil { - t.Fatalf("WriteSessionState for session1 failed: %v", err) - } - - // ======================================== - // Phase 4: New session 2 creates unrelated file - // ======================================== - t.Log("Phase 4: Session 2 creates file3.txt (unrelated to session 1)") - - session2 := env.NewSession() - if err := env.SimulateUserPromptSubmit(session2.ID); err != nil { - t.Fatalf("SimulateUserPromptSubmit for session2 failed: %v", err) - } - - env.WriteFile("file3.txt", "content from session 2") - session2.CreateTranscript( - "Create file3", - []FileChange{{Path: "file3.txt", Content: "content from session 2"}}, - ) - if err := env.SimulateStop(session2.ID, session2.TranscriptPath); err != nil { - t.Fatalf("SimulateStop for session2 failed: %v", err) - } - - // Set session2 to ACTIVE (simulating mid-turn commit) - state2, err := env.GetSessionState(session2.ID) - if err != nil { - t.Fatalf("GetSessionState for session2 failed: %v", err) - } - state2.Phase = session.PhaseActive - if err := env.WriteSessionState(session2.ID, state2); err != nil { - t.Fatalf("WriteSessionState for session2 failed: %v", err) - } - - // ======================================== - // Phase 5: Commit file3.txt (unrelated to session 1's files) - // ======================================== - t.Log("Phase 5: Committing file3.txt from session 2") - - env.GitAdd("file3.txt") - env.GitCommitWithShadowHooks("Add file3 from session 2", "file3.txt") - - finalHead := env.GetHeadHash() - t.Logf("Final HEAD: %s", finalHead[:7]) - - // ======================================== - // Phase 6: Verify session 1 was NOT condensed - // ======================================== - t.Log("Phase 6: Verifying session 1 was NOT condensed into unrelated commit") - - state1After, err := env.GetSessionState(session1.ID) - if err != nil { - t.Fatalf("GetSessionState for session1 after session2 commit failed: %v", err) - } - - // StepCount should be unchanged (condensation resets it) - if state1After.StepCount != session1StepCountAfterPartial { - t.Errorf("Session 1 StepCount changed! Expected %d, got %d (was incorrectly condensed)", - session1StepCountAfterPartial, state1After.StepCount) - } - - // BaseCommit should be unchanged (ENDED sessions don't get BaseCommit updated) - if state1After.BaseCommit != session1BaseCommitAfterPartial { - t.Errorf("Session 1 BaseCommit changed! Expected %s, got %s", - session1BaseCommitAfterPartial[:7], state1After.BaseCommit[:7]) - } - - // FilesTouched should still have file2.txt (not cleared by condensation) - hasFile2After := false - for _, f := range state1After.FilesTouched { - if f == "file2.txt" { - hasFile2After = true - break - } - } - if !hasFile2After { - t.Errorf("Session 1 FilesTouched was cleared! Expected file2.txt to remain, got: %v", - state1After.FilesTouched) - } - - // Phase should still be ENDED - if state1After.Phase != session.PhaseEnded { - t.Errorf("Session 1 phase changed! Expected ENDED, got %s", state1After.Phase) - } - - // ======================================== - // Phase 7: Verify session 2 WAS condensed - // ======================================== - t.Log("Phase 7: Verifying session 2 WAS condensed") - - state2After, err := env.GetSessionState(session2.ID) - if err != nil { - t.Fatalf("GetSessionState for session2 after commit failed: %v", err) - } - - // Session 2's BaseCommit should be updated to new HEAD - if state2After.BaseCommit != finalHead { - t.Errorf("Session 2 BaseCommit should be updated to new HEAD. Expected %s, got %s", - finalHead[:7], state2After.BaseCommit[:7]) - } - - t.Log("Test completed successfully") -} - -// TestCarryForward_NotCondensedIntoMultipleUnrelatedCommits verifies that a session -// with carry-forward files is NOT condensed into MULTIPLE subsequent unrelated commits. -// -// This is a regression test for the "repeat on every commit" bug where sessions -// with carry-forward files would be re-condensed into every subsequent commit -// indefinitely, polluting checkpoints. +// This integration test complements the unit tests in phase_postcommit_test.go by +// testing the full hook invocation path with multiple sessions interacting. // // Scenario: // 1. Session 1 creates file1.txt and file2.txt // 2. User commits only file1.txt (partial commit) // 3. Session 1 gets carry-forward: FilesTouched = ["file2.txt"] -// 4. Make 3 unrelated commits (file3.txt, file4.txt, file5.txt) -// 5. Verify: Session 1 was NOT condensed into ANY of those commits -// 6. Finally commit file2.txt -// 7. Verify: Session 1 IS condensed (carry-forward consumed) -func TestCarryForward_NotCondensedIntoMultipleUnrelatedCommits(t *testing.T) { - t.Parallel() - env := NewTestEnv(t) - defer env.Cleanup() - - // ======================================== - // Setup - // ======================================== - env.InitRepo() - env.WriteFile("README.md", "# Test Repository") - env.GitAdd("README.md") - env.GitCommit("Initial commit") - env.GitCheckoutNewBranch("feature/repeat-condensation-test") - env.InitEntire(strategy.StrategyNameManualCommit) - - // ======================================== - // Phase 1: Session 1 creates multiple files - // ======================================== - t.Log("Phase 1: Session 1 creates file1.txt and file2.txt") - - session1 := env.NewSession() - if err := env.SimulateUserPromptSubmit(session1.ID); err != nil { - t.Fatalf("SimulateUserPromptSubmit for session1 failed: %v", err) - } - - env.WriteFile("file1.txt", "content from session 1 - file 1") - env.WriteFile("file2.txt", "content from session 1 - file 2") - session1.CreateTranscript( - "Create file1 and file2", - []FileChange{ - {Path: "file1.txt", Content: "content from session 1 - file 1"}, - {Path: "file2.txt", Content: "content from session 1 - file 2"}, - }, - ) - if err := env.SimulateStop(session1.ID, session1.TranscriptPath); err != nil { - t.Fatalf("SimulateStop for session1 failed: %v", err) - } - - // ======================================== - // Phase 2: Partial commit - only file1.txt - // ======================================== - t.Log("Phase 2: Committing only file1.txt (partial commit)") - - env.GitAdd("file1.txt") - env.GitCommitWithShadowHooks("Partial commit: only file1", "file1.txt") - - // Verify carry-forward - state1, err := env.GetSessionState(session1.ID) - if err != nil { - t.Fatalf("GetSessionState for session1 after partial commit failed: %v", err) - } - t.Logf("Session1 FilesTouched after partial commit: %v", state1.FilesTouched) - - // End session 1 and force hasNew=true by setting CheckpointTranscriptStart=0 - // This simulates the bug scenario where sessionHasNewContent returns true - // because the transcript has "grown" since the (reset) checkpoint start. - state1.Phase = session.PhaseEnded - state1.CheckpointTranscriptStart = 0 // Force hasNew=true for subsequent commits - if err := env.WriteSessionState(session1.ID, state1); err != nil { - t.Fatalf("WriteSessionState for session1 failed: %v", err) - } - - // Record initial state for comparison - session1InitialStepCount := state1.StepCount - - // ======================================== - // Phase 3: Make multiple unrelated commits - // ======================================== - unrelatedFiles := []string{"file3.txt", "file4.txt", "file5.txt"} - - for i, fileName := range unrelatedFiles { - t.Logf("Phase 3.%d: Making unrelated commit with %s", i+1, fileName) - - env.WriteFile(fileName, "unrelated content "+fileName) - env.GitAdd(fileName) - env.GitCommitWithShadowHooks("Add "+fileName, fileName) - - // Verify session 1 was NOT condensed after each commit - state1, err = env.GetSessionState(session1.ID) - if err != nil { - t.Fatalf("GetSessionState for session1 after commit %d failed: %v", i+1, err) - } - - if state1.StepCount != session1InitialStepCount { - t.Errorf("After commit %d (%s): Session 1 StepCount changed from %d to %d (incorrectly condensed!)", - i+1, fileName, session1InitialStepCount, state1.StepCount) - } - - // FilesTouched should still have file2.txt - hasFile2 := false - for _, f := range state1.FilesTouched { - if f == "file2.txt" { - hasFile2 = true - break - } - } - if !hasFile2 { - t.Errorf("After commit %d (%s): Session 1 FilesTouched was cleared! Expected file2.txt, got: %v", - i+1, fileName, state1.FilesTouched) - } - - t.Logf(" -> Session 1 correctly NOT condensed (StepCount=%d, FilesTouched=%v)", - state1.StepCount, state1.FilesTouched) - } - - // ======================================== - // Phase 4: Finally commit file2.txt (the carry-forward file) - // ======================================== - t.Log("Phase 4: Committing file2.txt (the carry-forward file)") - - env.GitAdd("file2.txt") - env.GitCommitWithShadowHooks("Add file2 (carry-forward)", "file2.txt") - - // ======================================== - // Phase 5: Verify session 1 WAS condensed this time - // ======================================== - t.Log("Phase 5: Verifying session 1 WAS condensed when its file was committed") - - state1, err = env.GetSessionState(session1.ID) - if err != nil { - t.Fatalf("GetSessionState for session1 after file2 commit failed: %v", err) - } - - // StepCount should be reset (condensation happened) - if state1.StepCount != 0 { - t.Errorf("Session 1 StepCount should be 0 after condensation, got %d", state1.StepCount) - } - - // FilesTouched should be empty (carry-forward consumed) - if len(state1.FilesTouched) != 0 { - t.Errorf("Session 1 FilesTouched should be empty after condensation, got: %v", state1.FilesTouched) - } - - t.Log("Test completed successfully - session correctly condensed only when its file was committed") -} - -// TestCarryForward_NewSessionCommitDoesNotCondenseOldSession verifies that when -// an old session has carry-forward files and a NEW session commits unrelated files, -// the old session is NOT condensed into the new session's commit. -// -// This tests the interaction between multiple sessions where: -// 1. Old session has carry-forward files (file2.txt) -// 2. New session creates and commits different files (file6.txt) -// 3. Old session should remain untouched +// 4. Session 1 ends +// 5. Make some unrelated commits (simulating time passing) +// 6. New session 2 creates and commits file6.txt +// 7. Verify: Session 1 was NOT condensed into session 2's commit +// 8. Finally commit file2.txt +// 9. Verify: Session 1 IS condensed (carry-forward consumed) func TestCarryForward_NewSessionCommitDoesNotCondenseOldSession(t *testing.T) { t.Parallel() env := NewTestEnv(t) @@ -564,330 +218,3 @@ func TestCarryForward_NewSessionCommitDoesNotCondenseOldSession(t *testing.T) { t.Log(" - Session 1 NOT condensed into session 2's commit (file6.txt)") t.Log(" - Session 1 WAS condensed when its own file (file2.txt) was committed") } - -// TestStaleActiveSession_NotCondensedIntoUnrelatedCommit verifies that a stale -// ACTIVE session (agent resumed then killed) is NOT condensed into an unrelated -// commit from a different session. -// -// This is a regression test for the bug where ACTIVE sessions would always be -// condensed (hasNew=true) without checking file overlap. -// -// Scenario: -// 1. Session 1 creates file1.txt, Stop hook fires (checkpoint saved) -// 2. Session 1 is resumed (back to ACTIVE), does more work -// 3. Agent is killed (stays ACTIVE, new work not committed) -// 4. New session 2 creates file2.txt (unrelated) -// 5. Session 2 commits file2.txt -// 6. Verify: Session 1 was NOT condensed (its files weren't in the commit) -func TestStaleActiveSession_NotCondensedIntoUnrelatedCommit(t *testing.T) { - t.Parallel() - env := NewTestEnv(t) - defer env.Cleanup() - - // ======================================== - // Setup - // ======================================== - env.InitRepo() - env.WriteFile("README.md", "# Test Repository") - env.GitAdd("README.md") - env.GitCommit("Initial commit") - env.GitCheckoutNewBranch("feature/stale-active-test") - env.InitEntire(strategy.StrategyNameManualCommit) - - // ======================================== - // Phase 1: Session 1 creates file, Stop hook fires (creates checkpoint) - // ======================================== - t.Log("Phase 1: Session 1 creates file1.txt, Stop hook fires") - - session1 := env.NewSession() - if err := env.SimulateUserPromptSubmit(session1.ID); err != nil { - t.Fatalf("SimulateUserPromptSubmit for session1 failed: %v", err) - } - - env.WriteFile("file1.txt", "content from session 1") - session1.CreateTranscript( - "Create file1", - []FileChange{{Path: "file1.txt", Content: "content from session 1"}}, - ) - if err := env.SimulateStop(session1.ID, session1.TranscriptPath); err != nil { - t.Fatalf("SimulateStop for session1 failed: %v", err) - } - - // Session 1 is now IDLE with a checkpoint - state1, err := env.GetSessionState(session1.ID) - if err != nil { - t.Fatalf("GetSessionState for session1 failed: %v", err) - } - t.Logf("Session1 after Stop: Phase=%s, StepCount=%d, CheckpointTranscriptStart=%d", - state1.Phase, state1.StepCount, state1.CheckpointTranscriptStart) - - // ======================================== - // Phase 2: Simulate agent resume + crash (ACTIVE with hasNew=true) - // Set ACTIVE and reset CheckpointTranscriptStart to simulate: - // - Agent was resumed (ACTIVE) - // - Did more work (transcript grew, but not saved to checkpoint yet) - // - Then crashed (stuck ACTIVE with hasNew=true) - // ======================================== - t.Log("Phase 2: Simulating agent resume then crash (ACTIVE with hasNew=true)") - - state1.Phase = session.PhaseActive - state1.CheckpointTranscriptStart = 0 // Force hasNew=true (transcript > 0) - if err := env.WriteSessionState(session1.ID, state1); err != nil { - t.Fatalf("WriteSessionState for session1 failed: %v", err) - } - - // Record state for comparison - session1StepCount := state1.StepCount - t.Logf("Session1 (stale ACTIVE) StepCount: %d, CheckpointTranscriptStart: %d (forced to 0 for hasNew=true)", - session1StepCount, state1.CheckpointTranscriptStart) - - // ======================================== - // Phase 2b: Make an unrelated commit to move HEAD forward - // This ensures Session 1 and Session 2 have DIFFERENT shadow branches - // (different BaseCommit), isolating their condensation behavior. - // ======================================== - t.Log("Phase 2b: Making unrelated commit to separate shadow branches") - env.WriteFile("unrelated.txt", "unrelated content") - env.GitAdd("unrelated.txt") - env.GitCommit("Unrelated commit to move HEAD") - - // ======================================== - // Phase 3: New session 2 creates unrelated file - // ======================================== - t.Log("Phase 3: Session 2 creates file2.txt (unrelated to session 1)") - - session2 := env.NewSession() - if err := env.SimulateUserPromptSubmit(session2.ID); err != nil { - t.Fatalf("SimulateUserPromptSubmit for session2 failed: %v", err) - } - - env.WriteFile("file2.txt", "content from session 2") - session2.CreateTranscript( - "Create file2", - []FileChange{{Path: "file2.txt", Content: "content from session 2"}}, - ) - if err := env.SimulateStop(session2.ID, session2.TranscriptPath); err != nil { - t.Fatalf("SimulateStop for session2 failed: %v", err) - } - - // Set session2 to ACTIVE - state2, err := env.GetSessionState(session2.ID) - if err != nil { - t.Fatalf("GetSessionState for session2 failed: %v", err) - } - state2.Phase = session.PhaseActive - if err := env.WriteSessionState(session2.ID, state2); err != nil { - t.Fatalf("WriteSessionState for session2 failed: %v", err) - } - - // ======================================== - // Phase 4: Commit file2.txt (unrelated to session 1) - // ======================================== - t.Log("Phase 4: Committing file2.txt from session 2") - - env.GitAdd("file2.txt") - env.GitCommitWithShadowHooks("Add file2 from session 2", "file2.txt") - - finalHead := env.GetHeadHash() - t.Logf("Final HEAD: %s", finalHead[:7]) - - // ======================================== - // Phase 5: Verify stale session 1 was NOT condensed - // ======================================== - t.Log("Phase 5: Verifying stale session 1 was NOT condensed") - - state1After, err := env.GetSessionState(session1.ID) - if err != nil { - t.Fatalf("GetSessionState for session1 after session2 commit failed: %v", err) - } - - // StepCount should be unchanged (condensation resets it) - if state1After.StepCount != session1StepCount { - t.Errorf("Stale session 1 StepCount changed! Expected %d, got %d (was incorrectly condensed)", - session1StepCount, state1After.StepCount) - } - - // Note: BaseCommit WILL be updated for ACTIVE sessions (even stale ones) because - // updateBaseCommitIfChanged only skips IDLE/ENDED sessions. This is fine - the - // key fix is the overlap check that prevents condensation, not BaseCommit updates. - // A stale ACTIVE session that gets resumed later needs to know the current HEAD. - - // FilesTouched should still have file1.txt (not cleared by condensation) - hasFile1 := false - for _, f := range state1After.FilesTouched { - if f == "file1.txt" { - hasFile1 = true - break - } - } - if !hasFile1 { - t.Errorf("Stale session 1 FilesTouched was cleared! Expected file1.txt, got: %v", - state1After.FilesTouched) - } - - // ======================================== - // Phase 6: Verify session 2 WAS condensed - // ======================================== - t.Log("Phase 6: Verifying session 2 WAS condensed") - - state2After, err := env.GetSessionState(session2.ID) - if err != nil { - t.Fatalf("GetSessionState for session2 after commit failed: %v", err) - } - - if state2After.BaseCommit != finalHead { - t.Errorf("Session 2 BaseCommit should be updated to new HEAD. Expected %s, got %s", - finalHead[:7], state2After.BaseCommit[:7]) - } - - t.Log("Test completed successfully") -} - -// TestIdleSessionEmptyFilesTouched_NotCondensedIntoUnrelatedCommit verifies that -// an IDLE session with empty FilesTouched (e.g., conversation-only session) is NOT -// condensed into an unrelated commit. -// -// This is a regression test for the fail-open bug where shouldCondenseWithOverlapCheck -// would return true when filesTouchedBefore was empty, causing conversation-only -// sessions to be condensed into every commit. -// -// Scenario: -// 1. Session 1 has a conversation but doesn't modify any files -// 2. New session 2 creates file1.txt -// 3. Session 2 commits file1.txt -// 4. Verify: Session 1 was NOT condensed -func TestIdleSessionEmptyFilesTouched_NotCondensedIntoUnrelatedCommit(t *testing.T) { - t.Parallel() - env := NewTestEnv(t) - defer env.Cleanup() - - // ======================================== - // Setup - // ======================================== - env.InitRepo() - env.WriteFile("README.md", "# Test Repository") - env.GitAdd("README.md") - env.GitCommit("Initial commit") - env.GitCheckoutNewBranch("feature/empty-files-test") - env.InitEntire(strategy.StrategyNameManualCommit) - - // ======================================== - // Phase 1: Session 1 - conversation only, no file changes - // ======================================== - t.Log("Phase 1: Session 1 has conversation but no file changes") - - session1 := env.NewSession() - if err := env.SimulateUserPromptSubmit(session1.ID); err != nil { - t.Fatalf("SimulateUserPromptSubmit for session1 failed: %v", err) - } - - // Create transcript with NO file changes - session1.CreateTranscript( - "What is the meaning of life?", - []FileChange{}, // No files touched - ) - if err := env.SimulateStop(session1.ID, session1.TranscriptPath); err != nil { - t.Fatalf("SimulateStop for session1 failed: %v", err) - } - - // Verify session1 is IDLE with empty FilesTouched - state1, err := env.GetSessionState(session1.ID) - if err != nil { - t.Fatalf("GetSessionState for session1 failed: %v", err) - } - if state1.Phase != session.PhaseIdle { - t.Fatalf("Expected session1 to be IDLE, got %s", state1.Phase) - } - if len(state1.FilesTouched) != 0 { - t.Fatalf("Expected session1 FilesTouched to be empty, got: %v", state1.FilesTouched) - } - - session1StepCount := state1.StepCount - session1BaseCommit := state1.BaseCommit - t.Logf("Session1 (conversation-only) BaseCommit: %s, StepCount: %d", - session1BaseCommit[:7], session1StepCount) - - // ======================================== - // Phase 2: Session 2 creates a file - // ======================================== - t.Log("Phase 2: Session 2 creates file1.txt") - - session2 := env.NewSession() - if err := env.SimulateUserPromptSubmit(session2.ID); err != nil { - t.Fatalf("SimulateUserPromptSubmit for session2 failed: %v", err) - } - - env.WriteFile("file1.txt", "content from session 2") - session2.CreateTranscript( - "Create file1", - []FileChange{{Path: "file1.txt", Content: "content from session 2"}}, - ) - if err := env.SimulateStop(session2.ID, session2.TranscriptPath); err != nil { - t.Fatalf("SimulateStop for session2 failed: %v", err) - } - - // Set session2 to ACTIVE - state2, err := env.GetSessionState(session2.ID) - if err != nil { - t.Fatalf("GetSessionState for session2 failed: %v", err) - } - state2.Phase = session.PhaseActive - if err := env.WriteSessionState(session2.ID, state2); err != nil { - t.Fatalf("WriteSessionState for session2 failed: %v", err) - } - - // ======================================== - // Phase 3: Commit file1.txt - // ======================================== - t.Log("Phase 3: Committing file1.txt from session 2") - - env.GitAdd("file1.txt") - env.GitCommitWithShadowHooks("Add file1 from session 2", "file1.txt") - - finalHead := env.GetHeadHash() - - // ======================================== - // Phase 4: Verify conversation-only session 1 was NOT condensed - // ======================================== - t.Log("Phase 4: Verifying conversation-only session 1 was NOT condensed") - - state1After, err := env.GetSessionState(session1.ID) - if err != nil { - t.Fatalf("GetSessionState for session1 after session2 commit failed: %v", err) - } - - // StepCount should be unchanged - if state1After.StepCount != session1StepCount { - t.Errorf("Conversation-only session 1 StepCount changed! Expected %d, got %d", - session1StepCount, state1After.StepCount) - } - - // BaseCommit should be unchanged - if state1After.BaseCommit != session1BaseCommit { - t.Errorf("Conversation-only session 1 BaseCommit changed! Expected %s, got %s", - session1BaseCommit[:7], state1After.BaseCommit[:7]) - } - - // FilesTouched should still be empty - if len(state1After.FilesTouched) != 0 { - t.Errorf("Conversation-only session 1 FilesTouched should remain empty, got: %v", - state1After.FilesTouched) - } - - // ======================================== - // Phase 6: Verify session 2 WAS condensed - // ======================================== - t.Log("Phase 6: Verifying session 2 WAS condensed") - - state2After, err := env.GetSessionState(session2.ID) - if err != nil { - t.Fatalf("GetSessionState for session2 after commit failed: %v", err) - } - - if state2After.BaseCommit != finalHead { - t.Errorf("Session 2 BaseCommit should be updated to new HEAD. Expected %s, got %s", - finalHead[:7], state2After.BaseCommit[:7]) - } - - t.Log("Test completed successfully") -} From 9d09ac52b9e2a023b4cba3b781a448f922bc406c Mon Sep 17 00:00:00 2001 From: Stefan Haubold Date: Thu, 19 Feb 2026 13:27:08 +0100 Subject: [PATCH 7/7] handle deletes correctly Entire-Checkpoint: 1a2c8fa29e67 --- cmd/entire/cli/strategy/content_overlap.go | 20 +++- .../cli/strategy/content_overlap_test.go | 97 +++++++++++++++++++ 2 files changed, 112 insertions(+), 5 deletions(-) diff --git a/cmd/entire/cli/strategy/content_overlap.go b/cmd/entire/cli/strategy/content_overlap.go index 9319257ce..bb9ffdb65 100644 --- a/cmd/entire/cli/strategy/content_overlap.go +++ b/cmd/entire/cli/strategy/content_overlap.go @@ -98,11 +98,19 @@ func filesOverlapWithContent(repo *git.Repository, shadowBranchName string, head // Get file from HEAD tree (the committed content) headFile, err := headTree.File(filePath) if err != nil { - // File not in HEAD commit. This happens when: - // - The session created/modified the file but user deleted it before committing - // - The file was staged as a deletion (git rm) - // In both cases, the session's work on this file is not in the commit, - // so it doesn't contribute to overlap. Continue checking other files. + // File not in HEAD commit. Check if this is a deletion (existed in parent). + // Deletions count as overlap because the agent's action (deleting the file) + // is being committed - we want the session context linked to this commit. + if parentTree != nil { + if _, parentErr := parentTree.File(filePath); parentErr == nil { + // File existed in parent but not in HEAD - this is a deletion + logging.Debug(logCtx, "filesOverlapWithContent: deleted file counts as overlap", + slog.String("file", filePath), + ) + return true + } + } + // File didn't exist in parent either - not a deletion, skip continue } @@ -220,6 +228,8 @@ func stagedFilesOverlapWithContent(repo *git.Repository, shadowTree *object.Tree isModified := headErr == nil // Modified files always count as overlap (user edited session's work) + // This includes deletions - if file exists in HEAD and is being deleted, + // that's the agent's work being committed. if isModified { logging.Debug(logCtx, "stagedFilesOverlapWithContent: modified file counts as overlap", slog.String("file", stagedPath), diff --git a/cmd/entire/cli/strategy/content_overlap_test.go b/cmd/entire/cli/strategy/content_overlap_test.go index 4e6b7e890..b9d9396b8 100644 --- a/cmd/entire/cli/strategy/content_overlap_test.go +++ b/cmd/entire/cli/strategy/content_overlap_test.go @@ -178,6 +178,51 @@ func TestFilesOverlapWithContent_FileNotInCommit(t *testing.T) { assert.True(t, result, "File in commit with matching content should count as overlap") } +// TestFilesOverlapWithContent_DeletedFile tests that a deleted file +// (existed in parent, not in HEAD) DOES count as overlap. +// The agent's action of deleting the file is being committed. +func TestFilesOverlapWithContent_DeletedFile(t *testing.T) { + t.Parallel() + dir := setupGitRepo(t) + + repo, err := git.PlainOpen(dir) + require.NoError(t, err) + + // Create and commit a file that will be deleted + toDelete := filepath.Join(dir, "to_delete.txt") + require.NoError(t, os.WriteFile(toDelete, []byte("content to delete"), 0o644)) + + wt, err := repo.Worktree() + require.NoError(t, err) + _, err = wt.Add("to_delete.txt") + require.NoError(t, err) + _, err = wt.Commit("Add file to delete", &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@test.com", When: time.Now()}, + }) + require.NoError(t, err) + + // Create shadow branch (simulating agent work that includes the deletion) + createShadowBranchWithContent(t, repo, "del1234", "e3b0c4", map[string][]byte{ + "other.txt": []byte("other content"), + }) + + // Delete the file and commit the deletion + _, err = wt.Remove("to_delete.txt") + require.NoError(t, err) + deleteCommit, err := wt.Commit("Delete file", &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@test.com", When: time.Now()}, + }) + require.NoError(t, err) + + commit, err := repo.CommitObject(deleteCommit) + require.NoError(t, err) + + // Test: deleted file in filesTouched should count as overlap + shadowBranch := checkpoint.ShadowBranchNameForCommit("del1234", "e3b0c4") + result := filesOverlapWithContent(repo, shadowBranch, commit, []string{"to_delete.txt"}) + assert.True(t, result, "Deleted file should count as overlap (agent's deletion being committed)") +} + // TestFilesOverlapWithContent_NoShadowBranch tests fallback when shadow branch doesn't exist. func TestFilesOverlapWithContent_NoShadowBranch(t *testing.T) { t.Parallel() @@ -499,6 +544,58 @@ func TestStagedFilesOverlapWithContent_NoOverlap(t *testing.T) { assert.False(t, result, "Non-overlapping files should return false") } +// TestStagedFilesOverlapWithContent_DeletedFile tests that a deleted file +// (exists in HEAD but staged for deletion) DOES count as overlap. +// The agent's action of deleting the file is being committed, so the session +// context should be linked to this commit. +func TestStagedFilesOverlapWithContent_DeletedFile(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + repo, err := git.PlainInit(dir, false) + require.NoError(t, err) + + worktree, err := repo.Worktree() + require.NoError(t, err) + + // Create and commit a file that will be deleted + filePath := filepath.Join(dir, "to_delete.txt") + err = os.WriteFile(filePath, []byte("original content"), 0644) + require.NoError(t, err) + _, err = worktree.Add("to_delete.txt") + require.NoError(t, err) + _, err = worktree.Commit("Add to_delete.txt", &git.CommitOptions{ + Author: &object.Signature{ + Name: "Test", + Email: "test@test.com", + When: time.Now(), + }, + }) + require.NoError(t, err) + + // Create shadow branch (simulating agent work on the file) + createShadowBranchWithContent(t, repo, "mno7890", "e3b0c4", map[string][]byte{ + "to_delete.txt": []byte("agent modified content"), + }) + + // Stage the file for deletion (git rm) + _, err = worktree.Remove("to_delete.txt") + require.NoError(t, err) + + // Get shadow tree + shadowBranch := checkpoint.ShadowBranchNameForCommit("mno7890", "e3b0c4") + shadowRef, err := repo.Reference(plumbing.NewBranchReferenceName(shadowBranch), true) + require.NoError(t, err) + shadowCommit, err := repo.CommitObject(shadowRef.Hash()) + require.NoError(t, err) + shadowTree, err := shadowCommit.Tree() + require.NoError(t, err) + + // Deleted file SHOULD count as overlap - the agent's deletion is being committed + result := stagedFilesOverlapWithContent(repo, shadowTree, []string{"to_delete.txt"}, []string{"to_delete.txt"}) + assert.True(t, result, "Deleted file should count as overlap (agent's deletion being committed)") +} + // createShadowBranchWithContent creates a shadow branch with the given file contents. // This helper directly uses go-git APIs to avoid paths.RepoRoot() dependency. //