diff --git a/CLAUDE.md b/CLAUDE.md index eea040e9e..8e80bd71f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -327,7 +327,7 @@ All strategies implement: Sessions track their lifecycle through phases managed by a state machine in `session/phase.go`: -**Phases:** `ACTIVE`, `ACTIVE_COMMITTED`, `IDLE`, `ENDED` +**Phases:** `ACTIVE`, `IDLE`, `ENDED` **Events:** - `TurnStart` - Agent begins a turn (UserPromptSubmit hook) @@ -338,13 +338,12 @@ Sessions track their lifecycle through phases managed by a state machine in `ses **Key transitions:** - `IDLE + TurnStart → ACTIVE` - Agent starts working -- `ACTIVE + TurnEnd → IDLE` - Agent finishes turn -- `ACTIVE + GitCommit → ACTIVE_COMMITTED` - User commits while agent is working (condensation deferred) -- `ACTIVE_COMMITTED + TurnEnd → IDLE` - Agent finishes after commit (condense now) +- `ACTIVE + TurnEnd → IDLE` - Agent finishes turn; trailing transcript is appended to prior checkpoint if no new files were touched +- `ACTIVE + GitCommit → ACTIVE` - User commits while agent is working (condense immediately, migrate shadow branch) - `IDLE + GitCommit → IDLE` - User commits between turns (condense immediately) - `ENDED + GitCommit → ENDED` - Post-session commit (condense if files touched) -The state machine emits **actions** (e.g., `ActionCondense`, `ActionMigrateShadowBranch`, `ActionDeferCondensation`) that hook handlers dispatch to strategy-specific implementations. +The state machine emits **actions** (e.g., `ActionCondense`, `ActionMigrateShadowBranch`) that hook handlers dispatch to strategy-specific implementations. #### Metadata Structure diff --git a/cmd/entire/cli/checkpoint/checkpoint.go b/cmd/entire/cli/checkpoint/checkpoint.go index e79fe83bf..a5d521d6a 100644 --- a/cmd/entire/cli/checkpoint/checkpoint.go +++ b/cmd/entire/cli/checkpoint/checkpoint.go @@ -101,6 +101,12 @@ type Store interface { // ListCommitted lists all committed checkpoints. ListCommitted(ctx context.Context) ([]CommittedInfo, error) + + // UpdateCommitted appends transcript content to an existing committed checkpoint. + // Used for trailing transcript handling: post-commit conversation that belongs + // to the checkpoint but arrives after initial condensation. + // Returns ErrCheckpointNotFound if the checkpoint doesn't exist. + UpdateCommitted(ctx context.Context, opts UpdateCommittedOptions) error } // WriteTemporaryResult contains the result of writing a temporary checkpoint. @@ -283,6 +289,24 @@ type WriteCommittedOptions struct { SessionTranscriptPath string } +// UpdateCommittedOptions contains options for updating an existing committed checkpoint. +type UpdateCommittedOptions struct { + // CheckpointID identifies the checkpoint to update + CheckpointID id.CheckpointID + + // SessionID identifies which session slot to update within the checkpoint + SessionID string + + // Transcript contains additional transcript lines to append + Transcript []byte + + // Prompts contains additional prompts to append + Prompts []string + + // Context is the updated context (replaces existing) + Context []byte +} + // CommittedInfo contains summary information about a committed checkpoint. type CommittedInfo struct { // CheckpointID is the stable 12-hex-char identifier diff --git a/cmd/entire/cli/checkpoint/committed.go b/cmd/entire/cli/checkpoint/committed.go index 67cf6947c..af665da62 100644 --- a/cmd/entire/cli/checkpoint/committed.go +++ b/cmd/entire/cli/checkpoint/committed.go @@ -7,6 +7,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "log/slog" "os" "path/filepath" @@ -1008,6 +1009,230 @@ func (s *GitStore) UpdateSummary(ctx context.Context, checkpointID id.Checkpoint return nil } +// UpdateCommitted appends transcript content to an existing committed checkpoint. +// Used for trailing transcript handling: post-commit conversation that belongs +// to the checkpoint but arrives after initial condensation. +// Only appends transcript/prompts and replaces context. Does NOT re-run +// auto-summarization or update FilesTouched/TokenUsage. +// Returns ErrCheckpointNotFound if the checkpoint doesn't exist. +func (s *GitStore) UpdateCommitted(ctx context.Context, opts UpdateCommittedOptions) error { + _ = ctx // Reserved for future use + + if opts.CheckpointID.IsEmpty() { + return errors.New("invalid update options: checkpoint ID is required") + } + + // Ensure sessions branch exists + if err := s.ensureSessionsBranch(); err != nil { + return fmt.Errorf("failed to ensure sessions branch: %w", err) + } + + // Get current branch tip and flatten tree + ref, entries, err := s.getSessionsBranchEntries() + if err != nil { + return err + } + + // Read root CheckpointSummary to find the session slot + basePath := opts.CheckpointID.Path() + "/" + rootMetadataPath := basePath + paths.MetadataFileName + entry, exists := entries[rootMetadataPath] + if !exists { + return ErrCheckpointNotFound + } + + checkpointSummary, err := s.readSummaryFromBlob(entry.Hash) + if err != nil { + return fmt.Errorf("failed to read checkpoint summary: %w", err) + } + + // Find session index matching opts.SessionID + sessionIndex := -1 + for i := range len(checkpointSummary.Sessions) { + metaPath := fmt.Sprintf("%s%d/%s", basePath, i, paths.MetadataFileName) + if metaEntry, metaExists := entries[metaPath]; metaExists { + meta, metaErr := s.readMetadataFromBlob(metaEntry.Hash) + if metaErr == nil && meta.SessionID == opts.SessionID { + sessionIndex = i + break + } + } + } + if sessionIndex == -1 { + if len(checkpointSummary.Sessions) == 0 { + return ErrCheckpointNotFound + } + // Fall back to latest session; log so mismatches are diagnosable. + sessionIndex = len(checkpointSummary.Sessions) - 1 + logging.Debug(ctx, "UpdateCommitted: session ID not found, falling back to latest", + slog.String("session_id", opts.SessionID), + slog.String("checkpoint_id", string(opts.CheckpointID)), + slog.Int("fallback_index", sessionIndex), + ) + } + + sessionPath := fmt.Sprintf("%s%d/", basePath, sessionIndex) + + // Append transcript + if len(opts.Transcript) > 0 { + if err := s.appendTranscript(opts, sessionPath, entries); err != nil { + return fmt.Errorf("failed to append transcript: %w", err) + } + } + + // Append prompts + if len(opts.Prompts) > 0 { + if err := s.appendPrompts(opts.Prompts, sessionPath, entries); err != nil { + return fmt.Errorf("failed to append prompts: %w", err) + } + } + + // Replace context + if len(opts.Context) > 0 { + contextBlob, err := CreateBlobFromContent(s.repo, opts.Context) + if err != nil { + return fmt.Errorf("failed to create context blob: %w", err) + } + contextPath := sessionPath + paths.ContextFileName + entries[contextPath] = object.TreeEntry{ + Name: contextPath, + Mode: filemode.Regular, + Hash: contextBlob, + } + } + + // Build and commit + newTreeHash, err := BuildTreeFromEntries(s.repo, entries) + if err != nil { + return err + } + + authorName, authorEmail := getGitAuthorFromRepo(s.repo) + commitMsg := fmt.Sprintf("Append trailing transcript for checkpoint %s", opts.CheckpointID) + newCommitHash, err := s.createCommit(newTreeHash, ref.Hash(), commitMsg, authorName, authorEmail) + if err != nil { + return err + } + + refName := plumbing.NewBranchReferenceName(paths.MetadataBranchName) + newRef := plumbing.NewHashReference(refName, newCommitHash) + if err := s.repo.Storer.SetReference(newRef); err != nil { + return fmt.Errorf("failed to set branch reference: %w", err) + } + + return nil +} + +// appendTranscript reads the existing transcript blob, appends new content, and +// writes the combined result back. Also updates the content hash. +func (s *GitStore) appendTranscript(opts UpdateCommittedOptions, sessionPath string, entries map[string]object.TreeEntry) error { + transcriptPath := sessionPath + paths.TranscriptFileName + + // Read existing transcript (handle chunked format by looking for base file) + var existingContent []byte + if entry, exists := entries[transcriptPath]; exists { + blob, err := s.repo.BlobObject(entry.Hash) + if err != nil { + return fmt.Errorf("failed to read existing transcript blob: %w", err) + } + reader, err := blob.Reader() + if err != nil { + return fmt.Errorf("failed to get transcript reader: %w", err) + } + existingContent, err = io.ReadAll(reader) + _ = reader.Close() + if err != nil { + return fmt.Errorf("failed to read transcript content: %w", err) + } + } + + // Append new transcript with newline separator + var combined []byte + if len(existingContent) > 0 { + combined = existingContent + // Ensure existing ends with newline before appending + if combined[len(combined)-1] != '\n' { + combined = append(combined, '\n') + } + combined = append(combined, opts.Transcript...) + } else { + combined = opts.Transcript + } + + // Write combined transcript blob + blobHash, err := CreateBlobFromContent(s.repo, combined) + if err != nil { + return fmt.Errorf("failed to create transcript blob: %w", err) + } + entries[transcriptPath] = object.TreeEntry{ + Name: transcriptPath, + Mode: filemode.Regular, + Hash: blobHash, + } + + // Update content hash + contentHash := fmt.Sprintf("sha256:%x", sha256.Sum256(combined)) + hashBlob, err := CreateBlobFromContent(s.repo, []byte(contentHash)) + if err != nil { + return fmt.Errorf("failed to create content hash blob: %w", err) + } + hashPath := sessionPath + paths.ContentHashFileName + entries[hashPath] = object.TreeEntry{ + Name: hashPath, + Mode: filemode.Regular, + Hash: hashBlob, + } + + return nil +} + +// appendPrompts reads the existing prompt blob, appends new prompts with separator, +// and writes the combined result back. +func (s *GitStore) appendPrompts(newPrompts []string, sessionPath string, entries map[string]object.TreeEntry) error { + promptPath := sessionPath + paths.PromptFileName + + // Read existing prompts + var existingContent string + if entry, exists := entries[promptPath]; exists { + blob, err := s.repo.BlobObject(entry.Hash) + if err != nil { + return fmt.Errorf("failed to read existing prompt blob: %w", err) + } + reader, err := blob.Reader() + if err != nil { + return fmt.Errorf("failed to get prompt reader: %w", err) + } + data, err := io.ReadAll(reader) + _ = reader.Close() + if err != nil { + return fmt.Errorf("failed to read prompt content: %w", err) + } + existingContent = string(data) + } + + // Combine with separator + newContent := strings.Join(newPrompts, "\n\n---\n\n") + var combined string + if existingContent != "" { + combined = existingContent + "\n\n---\n\n" + newContent + } else { + combined = newContent + } + + // Write combined prompts blob + blobHash, err := CreateBlobFromContent(s.repo, []byte(combined)) + if err != nil { + return fmt.Errorf("failed to create prompt blob: %w", err) + } + entries[promptPath] = object.TreeEntry{ + Name: promptPath, + Mode: filemode.Regular, + Hash: blobHash, + } + + return nil +} + // ensureSessionsBranch ensures the entire/checkpoints/v1 branch exists. func (s *GitStore) ensureSessionsBranch() error { refName := plumbing.NewBranchReferenceName(paths.MetadataBranchName) diff --git a/cmd/entire/cli/checkpoint/committed_update_test.go b/cmd/entire/cli/checkpoint/committed_update_test.go new file mode 100644 index 000000000..53cfac89f --- /dev/null +++ b/cmd/entire/cli/checkpoint/committed_update_test.go @@ -0,0 +1,187 @@ +package checkpoint + +import ( + "context" + "errors" + "testing" + + "github.com/entireio/cli/cmd/entire/cli/checkpoint/id" +) + +func TestGitStore_UpdateCommitted_AppendsTranscript(t *testing.T) { + t.Parallel() + + repo, _ := setupBranchTestRepo(t) + store := NewGitStore(repo) + checkpointID := id.MustCheckpointID("a1b2c3d4e5f6") + sessionID := "test-session-trailing" + + // Phase 1: Write initial checkpoint with transcript, prompts, context + initialTranscript := []byte(`{"type":"human","message":{"content":"initial prompt"}} +{"type":"assistant","message":{"content":"initial response"}}`) + initialPrompts := []string{"initial prompt"} + initialContext := []byte("# Initial Context\n\nSome context here.\n") + + err := store.WriteCommitted(context.Background(), WriteCommittedOptions{ + CheckpointID: checkpointID, + SessionID: sessionID, + Strategy: "manual-commit", + Transcript: initialTranscript, + Prompts: initialPrompts, + Context: initialContext, + FilesTouched: []string{"file1.go"}, + AuthorName: "Test Author", + AuthorEmail: "test@example.com", + }) + if err != nil { + t.Fatalf("WriteCommitted() error = %v", err) + } + + // Verify initial content + content, err := store.ReadSessionContent(context.Background(), checkpointID, 0) + if err != nil { + t.Fatalf("ReadSessionContent() error = %v", err) + } + if content.Prompts != "initial prompt" { + t.Errorf("initial prompts = %q, want %q", content.Prompts, "initial prompt") + } + + // Phase 2: Append trailing transcript + trailingTranscript := []byte(`{"type":"human","message":{"content":"trailing question"}} +{"type":"assistant","message":{"content":"trailing answer"}}`) + trailingPrompts := []string{"trailing question"} + updatedContext := []byte("# Updated Context\n\nIncludes trailing conversation.\n") + + err = store.UpdateCommitted(context.Background(), UpdateCommittedOptions{ + CheckpointID: checkpointID, + SessionID: sessionID, + Transcript: trailingTranscript, + Prompts: trailingPrompts, + Context: updatedContext, + }) + if err != nil { + t.Fatalf("UpdateCommitted() error = %v", err) + } + + // Phase 3: Verify merged content + updated, err := store.ReadSessionContent(context.Background(), checkpointID, 0) + if err != nil { + t.Fatalf("ReadSessionContent() after update error = %v", err) + } + + // Transcript should contain both initial and trailing lines + if len(updated.Transcript) <= len(initialTranscript) { + t.Errorf("updated transcript length (%d) should be greater than initial (%d)", + len(updated.Transcript), len(initialTranscript)) + } + + // Prompts should contain both initial and trailing + if updated.Prompts != "initial prompt\n\n---\n\ntrailing question" { + t.Errorf("updated prompts = %q, want combined prompts", updated.Prompts) + } + + // Context should be replaced + if updated.Context != "# Updated Context\n\nIncludes trailing conversation.\n" { + t.Errorf("updated context = %q, want replacement context", updated.Context) + } + + // Verify metadata is NOT modified (FilesTouched, CheckpointsCount unchanged) + metadata := readLatestSessionMetadata(t, repo, checkpointID) + if len(metadata.FilesTouched) != 1 || metadata.FilesTouched[0] != "file1.go" { + t.Errorf("metadata.FilesTouched = %v, want [file1.go] (should be unchanged)", metadata.FilesTouched) + } +} + +func TestGitStore_UpdateCommitted_NotFound(t *testing.T) { + t.Parallel() + + repo, _ := setupBranchTestRepo(t) + store := NewGitStore(repo) + + // Ensure sessions branch exists + err := store.ensureSessionsBranch() + if err != nil { + t.Fatalf("ensureSessionsBranch() error = %v", err) + } + + // Try to update a non-existent checkpoint + checkpointID := id.MustCheckpointID("000000000000") + err = store.UpdateCommitted(context.Background(), UpdateCommittedOptions{ + CheckpointID: checkpointID, + SessionID: "nonexistent-session", + Transcript: []byte("trailing data"), + }) + if err == nil { + t.Error("UpdateCommitted() should return error for non-existent checkpoint") + } + if !errors.Is(err, ErrCheckpointNotFound) { + t.Errorf("UpdateCommitted() error = %v, want ErrCheckpointNotFound", err) + } +} + +func TestGitStore_UpdateCommitted_MatchesSessionID(t *testing.T) { + t.Parallel() + + repo, _ := setupBranchTestRepo(t) + store := NewGitStore(repo) + checkpointID := id.MustCheckpointID("b1c2d3e4f5a6") + + // Write initial checkpoint with two sessions + err := store.WriteCommitted(context.Background(), WriteCommittedOptions{ + CheckpointID: checkpointID, + SessionID: "session-1", + Strategy: "manual-commit", + Transcript: []byte(`{"type":"human","message":{"content":"s1 prompt"}}`), + Prompts: []string{"s1 prompt"}, + FilesTouched: []string{"file1.go"}, + AuthorName: "Test", + AuthorEmail: "test@test.com", + }) + if err != nil { + t.Fatalf("WriteCommitted() session-1 error = %v", err) + } + + // Write second session to same checkpoint + err = store.WriteCommitted(context.Background(), WriteCommittedOptions{ + CheckpointID: checkpointID, + SessionID: "session-2", + Strategy: "manual-commit", + Transcript: []byte(`{"type":"human","message":{"content":"s2 prompt"}}`), + Prompts: []string{"s2 prompt"}, + FilesTouched: []string{"file2.go"}, + AuthorName: "Test", + AuthorEmail: "test@test.com", + }) + if err != nil { + t.Fatalf("WriteCommitted() session-2 error = %v", err) + } + + // Update session-1 with trailing transcript + err = store.UpdateCommitted(context.Background(), UpdateCommittedOptions{ + CheckpointID: checkpointID, + SessionID: "session-1", + Transcript: []byte(`{"type":"assistant","message":{"content":"trailing for s1"}}`), + Prompts: []string{"trailing prompt for s1"}, + }) + if err != nil { + t.Fatalf("UpdateCommitted() error = %v", err) + } + + // Verify session-1 was updated + s1, err := store.ReadSessionContentByID(context.Background(), checkpointID, "session-1") + if err != nil { + t.Fatalf("ReadSessionContentByID() session-1 error = %v", err) + } + if s1.Prompts != "s1 prompt\n\n---\n\ntrailing prompt for s1" { + t.Errorf("session-1 prompts = %q, want combined", s1.Prompts) + } + + // Verify session-2 was NOT modified + s2, err := store.ReadSessionContentByID(context.Background(), checkpointID, "session-2") + if err != nil { + t.Fatalf("ReadSessionContentByID() session-2 error = %v", err) + } + if s2.Prompts != "s2 prompt" { + t.Errorf("session-2 prompts = %q, want unchanged", s2.Prompts) + } +} diff --git a/cmd/entire/cli/doctor.go b/cmd/entire/cli/doctor.go index f4103f706..1b5a0a940 100644 --- a/cmd/entire/cli/doctor.go +++ b/cmd/entire/cli/doctor.go @@ -28,7 +28,7 @@ func newDoctorCmd() *cobra.Command { Long: `Scan for stuck or problematic sessions and offer to fix them. A session is considered stuck if: - - It is in ACTIVE or ACTIVE_COMMITTED phase with no interaction for over 1 hour + - It is in ACTIVE phase with no interaction for over 1 hour - It is in ENDED phase with uncondensed checkpoint data on a shadow branch For each stuck session, you can choose to: diff --git a/cmd/entire/cli/doctor_test.go b/cmd/entire/cli/doctor_test.go index ce5f9599f..046701c97 100644 --- a/cmd/entire/cli/doctor_test.go +++ b/cmd/entire/cli/doctor_test.go @@ -94,7 +94,6 @@ func TestClassifySession_ActiveStale_OldInteractionTime(t *testing.T) { assert.Equal(t, 2, result.FilesTouchedCount) } -//nolint:dupl // Tests distinct phase (ACTIVE vs ACTIVE_COMMITTED), collapsing harms readability func TestClassifySession_ActiveRecent_Healthy(t *testing.T) { dir := setupGitRepoForPhaseTest(t) repo, err := git.PlainOpen(dir) @@ -113,43 +112,42 @@ func TestClassifySession_ActiveRecent_Healthy(t *testing.T) { assert.Nil(t, result, "active session with recent interaction should be healthy") } -func TestClassifySession_ActiveCommittedStale(t *testing.T) { +func TestClassifySession_ActiveStale_HighStepCount(t *testing.T) { dir := setupGitRepoForPhaseTest(t) repo, err := git.PlainOpen(dir) require.NoError(t, err) twoHoursAgo := time.Now().Add(-2 * time.Hour) state := &strategy.SessionState{ - SessionID: "test-ac-stale", + SessionID: "test-active-stale-high-steps", BaseCommit: testBaseCommit, - Phase: session.PhaseActiveCommitted, + Phase: session.PhaseActive, StepCount: 5, LastInteractionTime: &twoHoursAgo, } result := classifySession(state, repo, time.Now()) - require.NotNil(t, result, "ACTIVE_COMMITTED session with stale interaction should be stuck") + require.NotNil(t, result, "ACTIVE session with stale interaction should be stuck") assert.Contains(t, result.Reason, "active, last interaction") } -//nolint:dupl // Tests distinct phase (ACTIVE_COMMITTED vs ACTIVE), collapsing harms readability -func TestClassifySession_ActiveCommittedRecent_Healthy(t *testing.T) { +func TestClassifySession_ActiveRecentHighSteps_Healthy(t *testing.T) { dir := setupGitRepoForPhaseTest(t) repo, err := git.PlainOpen(dir) require.NoError(t, err) recentTime := time.Now().Add(-30 * time.Minute) state := &strategy.SessionState{ - SessionID: "test-ac-healthy", + SessionID: "test-active-recent-high-steps", BaseCommit: testBaseCommit, - Phase: session.PhaseActiveCommitted, + Phase: session.PhaseActive, StepCount: 5, LastInteractionTime: &recentTime, } result := classifySession(state, repo, time.Now()) - assert.Nil(t, result, "ACTIVE_COMMITTED session with recent interaction should be healthy") + assert.Nil(t, result, "ACTIVE session with recent interaction should be healthy") } func TestClassifySession_EndedWithUncondensedData(t *testing.T) { diff --git a/cmd/entire/cli/hooks.go b/cmd/entire/cli/hooks.go index 363b528f1..005ae8c5c 100644 --- a/cmd/entire/cli/hooks.go +++ b/cmd/entire/cli/hooks.go @@ -280,7 +280,7 @@ func handleSessionStartCommon() error { // Fire EventSessionStart for the current session (if state exists). // This handles ENDED → IDLE (re-entering a session). - // TODO(ENT-221): dispatch ActionWarnStaleSession for ACTIVE/ACTIVE_COMMITTED sessions. + // TODO(ENT-221): dispatch ActionWarnStaleSession for ACTIVE sessions. if state, loadErr := strategy.LoadSessionState(input.SessionID); loadErr != nil { fmt.Fprintf(os.Stderr, "Warning: failed to load session state on start: %v\n", loadErr) } else if state != nil { diff --git a/cmd/entire/cli/hooks_claudecode_handlers.go b/cmd/entire/cli/hooks_claudecode_handlers.go index 450821f2e..998988a61 100644 --- a/cmd/entire/cli/hooks_claudecode_handlers.go +++ b/cmd/entire/cli/hooks_claudecode_handlers.go @@ -383,8 +383,7 @@ func commitWithMetadata() error { //nolint:maintidx // already present in codeba } // Fire EventTurnEnd to transition session phase (all strategies). - // This moves ACTIVE → IDLE or ACTIVE_COMMITTED → IDLE. - // For ACTIVE_COMMITTED → IDLE, HandleTurnEnd dispatches ActionCondense. + // This moves ACTIVE → IDLE. transitionSessionTurnEnd(sessionID) // Clean up pre-prompt state (CLI responsibility) @@ -733,8 +732,8 @@ func handleClaudeCodeSessionEnd() error { } // transitionSessionTurnEnd fires EventTurnEnd to move the session from -// ACTIVE → IDLE (or ACTIVE_COMMITTED → IDLE). Best-effort: logs warnings -// on failure rather than returning errors. +// ACTIVE → IDLE. Best-effort: logs warnings on failure rather than +// returning errors. func transitionSessionTurnEnd(sessionID string) { turnState, loadErr := strategy.LoadSessionState(sessionID) if loadErr != nil { @@ -746,13 +745,13 @@ func transitionSessionTurnEnd(sessionID string) { } remaining := strategy.TransitionAndLog(turnState, session.EventTurnEnd, session.TransitionContext{}) - // Dispatch strategy-specific actions (e.g., ActionCondense for ACTIVE_COMMITTED → IDLE) - if len(remaining) > 0 { - strat := GetStrategy() - if handler, ok := strat.(strategy.TurnEndHandler); ok { - if err := handler.HandleTurnEnd(turnState, remaining); err != nil { - fmt.Fprintf(os.Stderr, "Warning: turn-end action dispatch failed: %v\n", err) - } + // Always invoke strategy-specific turn-end handling. The handler may + // dispatch remaining actions AND perform turn-end housekeeping such as + // appending trailing transcripts to committed checkpoints. + strat := GetStrategy() + if handler, ok := strat.(strategy.TurnEndHandler); ok { + if err := handler.HandleTurnEnd(turnState, remaining); err != nil { + fmt.Fprintf(os.Stderr, "Warning: turn-end action dispatch failed: %v\n", err) } } diff --git a/cmd/entire/cli/integration_test/hooks.go b/cmd/entire/cli/integration_test/hooks.go index b748203b0..a519d1c1e 100644 --- a/cmd/entire/cli/integration_test/hooks.go +++ b/cmd/entire/cli/integration_test/hooks.go @@ -246,6 +246,20 @@ func (s *Session) CreateTranscript(prompt string, changes []FileChange) string { return s.TranscriptPath } +// SimulateUserPromptSubmitWithTranscript simulates the UserPromptSubmit hook +// with a transcript path. This is needed for tests that rely on InitializeSession +// updating state.TranscriptPath (e.g., trailing transcript tests). +func (r *HookRunner) SimulateUserPromptSubmitWithTranscript(sessionID, transcriptPath string) error { + r.T.Helper() + + input := map[string]string{ + "session_id": sessionID, + "transcript_path": transcriptPath, + } + + return r.runHookWithInput("user-prompt-submit", input) +} + // SimulateUserPromptSubmit is a convenience method on TestEnv. func (env *TestEnv) SimulateUserPromptSubmit(sessionID string) error { env.T.Helper() @@ -253,6 +267,13 @@ func (env *TestEnv) SimulateUserPromptSubmit(sessionID string) error { return runner.SimulateUserPromptSubmit(sessionID) } +// SimulateUserPromptSubmitWithTranscript is a convenience method on TestEnv. +func (env *TestEnv) SimulateUserPromptSubmitWithTranscript(sessionID, transcriptPath string) error { + env.T.Helper() + runner := NewHookRunner(env.RepoDir, env.ClaudeProjectDir, env.T) + return runner.SimulateUserPromptSubmitWithTranscript(sessionID, transcriptPath) +} + // SimulateUserPromptSubmitWithResponse is a convenience method on TestEnv. func (env *TestEnv) SimulateUserPromptSubmitWithResponse(sessionID string) (*HookResponse, error) { env.T.Helper() diff --git a/cmd/entire/cli/integration_test/hooks_test.go b/cmd/entire/cli/integration_test/hooks_test.go index e732100c8..b38c2ffe6 100644 --- a/cmd/entire/cli/integration_test/hooks_test.go +++ b/cmd/entire/cli/integration_test/hooks_test.go @@ -7,7 +7,7 @@ import ( "path/filepath" "testing" - "github.com/entireio/cli/cmd/entire/cli/strategy" + "github.com/entireio/cli/cmd/entire/cli/strategy" ) func TestHookRunner_SimulateUserPromptSubmit(t *testing.T) { diff --git a/cmd/entire/cli/integration_test/phase_transitions_test.go b/cmd/entire/cli/integration_test/phase_transitions_test.go index a1f00b1ed..8f2c249d4 100644 --- a/cmd/entire/cli/integration_test/phase_transitions_test.go +++ b/cmd/entire/cli/integration_test/phase_transitions_test.go @@ -17,14 +17,14 @@ import ( // TestShadow_CommitBeforeStop tests the "commit while agent is still working" flow. // // When the user commits while the agent is in the ACTIVE phase (between -// SimulateUserPromptSubmit and SimulateStop), the session should transition to -// ACTIVE_COMMITTED. This defers condensation because the agent is still working. -// When the agent finishes its turn (SimulateStop), the deferred condensation fires -// and the session transitions to IDLE with metadata persisted to entire/checkpoints/v1. +// SimulateUserPromptSubmit and SimulateStop), condensation happens immediately +// in PostCommit. The phase stays ACTIVE (no ACTIVE_COMMITTED transition). +// The shadow branch is migrated to the new HEAD after condensation. +// When the agent finishes its turn (SimulateStop), the session transitions to IDLE. // // State machine transitions tested: -// - ACTIVE + GitCommit -> ACTIVE_COMMITTED (defer condensation, migrate shadow branch) -// - ACTIVE_COMMITTED + TurnEnd -> IDLE + ActionCondense (deferred condensation fires) +// - ACTIVE + GitCommit -> ACTIVE + [ActionCondense, ActionMigrateShadowBranch] +// - ACTIVE + TurnEnd -> IDLE func TestShadow_CommitBeforeStop(t *testing.T) { t.Parallel() @@ -147,7 +147,7 @@ func TestShadow_CommitBeforeStop(t *testing.T) { t.Logf("Commit has checkpoint trailer: %s", checkpointID) } - // CRITICAL: Verify session phase is ACTIVE_COMMITTED + // CRITICAL: Verify session phase stays ACTIVE (condenses immediately, no deferral) state, err = env.GetSessionState(sess.ID) if err != nil { t.Fatalf("GetSessionState failed: %v", err) @@ -155,17 +155,16 @@ func TestShadow_CommitBeforeStop(t *testing.T) { if state == nil { t.Fatal("Session state should exist after commit") } - if state.Phase != session.PhaseActiveCommitted { + if state.Phase != session.PhaseActive { t.Errorf("Phase after commit-while-active should be %q, got %q", - session.PhaseActiveCommitted, state.Phase) + session.PhaseActive, state.Phase) } t.Logf("Session phase after mid-turn commit: %s", state.Phase) - // Verify shadow branch was migrated to the new HEAD - // The old shadow branch (based on initialHead) may still exist or be cleaned up. - // The important thing is that the session's BaseCommit was updated. - if state.BaseCommit == initialHead { - t.Logf("Note: BaseCommit not yet updated (may happen during migration)") + // Condensation happens immediately during PostCommit for ACTIVE sessions. + // Verify StepCount was reset and shadow branch was migrated. + if state.StepCount != 0 { + t.Errorf("StepCount should be 0 after immediate condensation, got %d", state.StepCount) } // ======================================== @@ -186,19 +185,13 @@ func TestShadow_CommitBeforeStop(t *testing.T) { t.Fatal("Session state should exist after stop") } if state.Phase != session.PhaseIdle { - t.Errorf("Phase after stop from ACTIVE_COMMITTED should be %q, got %q", + t.Errorf("Phase after stop from ACTIVE should be %q, got %q", session.PhaseIdle, state.Phase) } t.Logf("Session phase after stop: %s (StepCount: %d)", state.Phase, state.StepCount) - // Deferred condensation should have fired during TurnEnd (ACTIVE_COMMITTED → IDLE). - // Verify StepCount was reset and metadata was persisted to entire/checkpoints/v1. - if state.StepCount != 0 { - t.Errorf("StepCount should be 0 after TurnEnd condensation, got %d", state.StepCount) - } - if !env.BranchExists(paths.MetadataBranchName) { - t.Fatal("entire/checkpoints/v1 branch should exist after TurnEnd condensation") + t.Fatal("entire/checkpoints/v1 branch should exist after PostCommit condensation") } latestCheckpointID := env.TryGetLatestCheckpointID() if latestCheckpointID != "" { diff --git a/cmd/entire/cli/integration_test/trailing_transcript_test.go b/cmd/entire/cli/integration_test/trailing_transcript_test.go new file mode 100644 index 000000000..f8725f200 --- /dev/null +++ b/cmd/entire/cli/integration_test/trailing_transcript_test.go @@ -0,0 +1,296 @@ +//go:build integration + +package integration + +import ( + "strings" + "testing" + + "github.com/entireio/cli/cmd/entire/cli/paths" + "github.com/entireio/cli/cmd/entire/cli/strategy" +) + +// TestShadow_TrailingTranscriptAppendedToCheckpoint tests that when an agent +// continues a conversation after a commit (condensation) without touching any +// new files, the trailing transcript is appended to the existing checkpoint. +// +// Scenario: +// 1. Start session, create a file, save checkpoint (SimulateStop) +// 2. User commits -> condensation creates checkpoint on entire/checkpoints/v1 +// 3. Agent continues conversation (new prompt + response in transcript, no file edits) +// 4. SimulateStop -> HandleTurnEnd -> handleTrailingTranscript appends to checkpoint +// 5. Verify checkpoint transcript and prompts contain the trailing conversation +func TestShadow_TrailingTranscriptAppendedToCheckpoint(t *testing.T) { + t.Parallel() + + env := NewTestEnv(t) + defer env.Cleanup() + + // Setup repository + env.InitRepo() + env.WriteFile("README.md", "# Test Repository") + env.GitAdd("README.md") + env.GitCommit("Initial commit") + env.GitCheckoutNewBranch("feature/trailing-transcript") + env.InitEntire(strategy.StrategyNameManualCommit) + + // ======================================== + // Phase 1: Start session and create checkpoint + // ======================================== + t.Log("Phase 1: Start session and create checkpoint") + + session := env.NewSession() + // Pass transcript path so InitializeSession sets state.TranscriptPath + // (needed later for trailing transcript detection) + if err := env.SimulateUserPromptSubmitWithTranscript(session.ID, session.TranscriptPath); err != nil { + t.Fatalf("SimulateUserPromptSubmit failed: %v", err) + } + + // Create a file (this counts as "files touched") + fileContent := "package main\n\nfunc Hello() string {\n\treturn \"hello\"\n}\n" + env.WriteFile("hello.go", fileContent) + + // Build transcript with file edit + session.TranscriptBuilder.AddUserMessage("Create a hello function in hello.go") + session.TranscriptBuilder.AddAssistantMessage("I'll create the hello function for you.") + toolID := session.TranscriptBuilder.AddToolUse("mcp__acp__Write", "hello.go", fileContent) + session.TranscriptBuilder.AddToolResult(toolID) + session.TranscriptBuilder.AddAssistantMessage("Done! I created hello.go with the Hello() function.") + + if err := session.TranscriptBuilder.WriteToFile(session.TranscriptPath); err != nil { + t.Fatalf("Failed to write transcript: %v", err) + } + + // Save checkpoint (SimulateStop triggers SaveChanges -> creates shadow branch checkpoint) + if err := env.SimulateStop(session.ID, session.TranscriptPath); err != nil { + t.Fatalf("SimulateStop failed: %v", err) + } + + // Verify checkpoint exists + rewindPoints := env.GetRewindPoints() + if len(rewindPoints) != 1 { + t.Fatalf("Expected 1 rewind point after first checkpoint, got %d", len(rewindPoints)) + } + t.Logf("Checkpoint 1 created: %s", rewindPoints[0].Message) + + // ======================================== + // Phase 2: User commits -> condensation + // ======================================== + t.Log("Phase 2: User commits - triggering condensation") + + env.GitCommitWithShadowHooks("Add hello function", "hello.go") + + commitHash := env.GetHeadHash() + checkpointID := env.GetCheckpointIDFromCommitMessage(commitHash) + if checkpointID == "" { + t.Fatal("Commit should have Entire-Checkpoint trailer after condensation") + } + t.Logf("Checkpoint ID from commit: %s", checkpointID) + + // Verify condensation happened - checkpoint exists on entire/checkpoints/v1 + if !env.BranchExists(paths.MetadataBranchName) { + t.Fatal("entire/checkpoints/v1 branch should exist after condensation") + } + + // Read the initial transcript from the checkpoint + transcriptPath := SessionFilePath(checkpointID, paths.TranscriptFileName) + initialTranscript, found := env.ReadFileFromBranch(paths.MetadataBranchName, transcriptPath) + if !found { + t.Fatalf("Transcript should exist at %s after condensation", transcriptPath) + } + t.Logf("Initial transcript length: %d bytes", len(initialTranscript)) + + // Read initial prompts + promptPath := SessionFilePath(checkpointID, "prompt.txt") + initialPrompts, found := env.ReadFileFromBranch(paths.MetadataBranchName, promptPath) + if !found { + t.Fatalf("Prompts should exist at %s after condensation", promptPath) + } + t.Logf("Initial prompts:\n%s", initialPrompts) + + // Verify session state after condensation + state, err := env.GetSessionState(session.ID) + if err != nil { + t.Fatalf("GetSessionState failed: %v", err) + } + if state == nil { + t.Fatal("Session state should exist after condensation") + } + if state.LastCheckpointID.IsEmpty() { + t.Error("LastCheckpointID should be set after condensation") + } + if state.StepCount != 0 { + t.Errorf("StepCount should be 0 after condensation, got %d", state.StepCount) + } + t.Logf("Session state after condensation: LastCheckpointID=%s, StepCount=%d, CheckpointTranscriptStart=%d", + state.LastCheckpointID, state.StepCount, state.CheckpointTranscriptStart) + + // ======================================== + // Phase 3: Agent continues with trailing conversation (no file edits) + // ======================================== + t.Log("Phase 3: Agent continues conversation without file edits") + + // Add trailing conversation to transcript BEFORE the next UserPromptSubmit. + // This simulates the agent explaining what it did after the commit, + // which adds conversation to the transcript without any file edits. + // The trailing content is already on disk when UserPromptSubmit fires. + session.TranscriptBuilder.AddUserMessage("Can you explain what the Hello function does?") + session.TranscriptBuilder.AddAssistantMessage("The Hello function returns the string \"hello\". It takes no parameters and returns a string type.") + + // Write the updated transcript (includes original + trailing content) + if err := session.TranscriptBuilder.WriteToFile(session.TranscriptPath); err != nil { + t.Fatalf("Failed to write updated transcript: %v", err) + } + + // ======================================== + // Phase 4: SimulateUserPromptSubmit triggers InitializeSession -> + // handleTrailingTranscript appends BEFORE clearing LastCheckpointID + // ======================================== + t.Log("Phase 4: UserPromptSubmit - should append trailing transcript before clearing checkpoint ID") + + // Pass transcript path so InitializeSession can read the trailing content + if err := env.SimulateUserPromptSubmitWithTranscript(session.ID, session.TranscriptPath); err != nil { + t.Fatalf("SimulateUserPromptSubmit (trailing) failed: %v", err) + } + + // ======================================== + // Phase 5: Verify trailing transcript was appended + // ======================================== + t.Log("Phase 5: Verifying trailing transcript was appended to checkpoint") + + // Read the transcript again from the checkpoint + updatedTranscript, found := env.ReadFileFromBranch(paths.MetadataBranchName, transcriptPath) + if !found { + t.Fatalf("Transcript should still exist at %s after trailing append", transcriptPath) + } + + // Transcript should be longer than before + if len(updatedTranscript) <= len(initialTranscript) { + t.Errorf("Transcript should be longer after trailing append: initial=%d, updated=%d", + len(initialTranscript), len(updatedTranscript)) + } else { + t.Logf("Transcript grew from %d to %d bytes", len(initialTranscript), len(updatedTranscript)) + } + + // The trailing conversation should be in the updated transcript + if !strings.Contains(updatedTranscript, "explain what the Hello function does") { + t.Error("Updated transcript should contain trailing user prompt") + } + + // Read the prompts again + updatedPrompts, found := env.ReadFileFromBranch(paths.MetadataBranchName, promptPath) + if !found { + t.Fatalf("Prompts should still exist at %s after trailing append", promptPath) + } + + // Prompts should include the trailing prompt + if !strings.Contains(updatedPrompts, "explain what the Hello function does") { + t.Error("Updated prompts should contain trailing user prompt") + } + t.Logf("Updated prompts:\n%s", updatedPrompts) + + // Verify session state was updated + state, err = env.GetSessionState(session.ID) + if err != nil { + t.Fatalf("GetSessionState (after trailing) failed: %v", err) + } + t.Logf("Session state after trailing: CheckpointTranscriptStart=%d", state.CheckpointTranscriptStart) + + t.Log("Trailing transcript test completed successfully!") +} + +// TestShadow_TrailingTranscriptSkippedWhenNewFilesTouched tests that trailing +// transcript handling is skipped when the agent has created new checkpoints +// (i.e., touched new files) after condensation. +// +// This ensures trailing transcript only fires when StepCount == 0 (no new files +// touched since the last condensation). +func TestShadow_TrailingTranscriptSkippedWhenNewFilesTouched(t *testing.T) { + t.Parallel() + + env := NewTestEnv(t) + defer env.Cleanup() + + // Setup repository + env.InitRepo() + env.WriteFile("README.md", "# Test Repository") + env.GitAdd("README.md") + env.GitCommit("Initial commit") + env.GitCheckoutNewBranch("feature/trailing-skip") + env.InitEntire(strategy.StrategyNameManualCommit) + + // ======================================== + // Phase 1: Start session, create checkpoint, commit + // ======================================== + t.Log("Phase 1: Start session, create checkpoint, commit") + + session := env.NewSession() + if err := env.SimulateUserPromptSubmit(session.ID); err != nil { + t.Fatalf("SimulateUserPromptSubmit failed: %v", err) + } + + fileContent := "package main\n\nfunc A() {}\n" + env.WriteFile("a.go", fileContent) + session.CreateTranscript("Create function A", []FileChange{{Path: "a.go", Content: fileContent}}) + + if err := env.SimulateStop(session.ID, session.TranscriptPath); err != nil { + t.Fatalf("SimulateStop failed: %v", err) + } + + // Commit to trigger condensation + env.GitCommitWithShadowHooks("Add function A", "a.go") + + checkpointID := env.GetCheckpointIDFromCommitMessage(env.GetHeadHash()) + if checkpointID == "" { + t.Fatal("Commit should have checkpoint trailer") + } + + // Read initial transcript + transcriptPath := SessionFilePath(checkpointID, paths.TranscriptFileName) + initialTranscript, found := env.ReadFileFromBranch(paths.MetadataBranchName, transcriptPath) + if !found { + t.Fatalf("Initial transcript should exist") + } + + // ======================================== + // Phase 2: Continue session WITH new file edits (StepCount > 0) + // ======================================== + t.Log("Phase 2: Continue with new file edits") + + if err := env.SimulateUserPromptSubmit(session.ID); err != nil { + t.Fatalf("SimulateUserPromptSubmit (continuing) failed: %v", err) + } + + // Create a NEW file (this means SaveChanges will create a new checkpoint) + fileBContent := "package main\n\nfunc B() {}\n" + env.WriteFile("b.go", fileBContent) + + // Reset transcript builder for the new turn + session.TranscriptBuilder = NewTranscriptBuilder() + session.CreateTranscript("Create function B", []FileChange{{Path: "b.go", Content: fileBContent}}) + + // This Stop creates a NEW checkpoint (StepCount becomes > 0) + if err := env.SimulateStop(session.ID, session.TranscriptPath); err != nil { + t.Fatalf("SimulateStop (with new files) failed: %v", err) + } + + // ======================================== + // Phase 3: Verify trailing transcript was NOT appended + // ======================================== + t.Log("Phase 3: Verify trailing transcript was not appended to old checkpoint") + + // The original checkpoint transcript should be unchanged + unchangedTranscript, found := env.ReadFileFromBranch(paths.MetadataBranchName, transcriptPath) + if !found { + t.Fatalf("Transcript should still exist") + } + + if unchangedTranscript != initialTranscript { + t.Errorf("Checkpoint transcript should not change when new files are touched.\nInitial length: %d\nCurrent length: %d", + len(initialTranscript), len(unchangedTranscript)) + } else { + t.Log("Correctly skipped trailing transcript when new files were touched (StepCount > 0)") + } + + t.Log("Trailing transcript skip test completed successfully!") +} diff --git a/cmd/entire/cli/phase_wiring_test.go b/cmd/entire/cli/phase_wiring_test.go index 8c79d06e0..9454ce0ac 100644 --- a/cmd/entire/cli/phase_wiring_test.go +++ b/cmd/entire/cli/phase_wiring_test.go @@ -68,8 +68,10 @@ func TestMarkSessionEnded_IdleToEnded(t *testing.T) { require.NotNil(t, loaded.EndedAt) } -// TestMarkSessionEnded_ActiveCommittedToEnded verifies ACTIVE_COMMITTED → ENDED. -func TestMarkSessionEnded_ActiveCommittedToEnded(t *testing.T) { +// TestMarkSessionEnded_OldActiveCommittedToEnded verifies backward compatibility: +// if a state file from an older CLI has phase="active_committed", it normalizes +// to ACTIVE and then transitions to ENDED. +func TestMarkSessionEnded_OldActiveCommittedToEnded(t *testing.T) { dir := setupGitRepoForPhaseTest(t) t.Chdir(dir) @@ -77,7 +79,7 @@ func TestMarkSessionEnded_ActiveCommittedToEnded(t *testing.T) { SessionID: "test-session-end-ac", BaseCommit: "abc123", StartedAt: time.Now(), - Phase: session.PhaseActiveCommitted, + Phase: "active_committed", // raw string simulating old state file } err := strategy.SaveSessionState(state) require.NoError(t, err) diff --git a/cmd/entire/cli/reset.go b/cmd/entire/cli/reset.go index ae6ccd06d..89a2a00a1 100644 --- a/cmd/entire/cli/reset.go +++ b/cmd/entire/cli/reset.go @@ -161,7 +161,7 @@ func runResetSession(cmd *cobra.Command, resetter strategy.SessionResetter, sess } // activeSessionsOnCurrentHead returns sessions on the current HEAD -// that are in an active phase (ACTIVE or ACTIVE_COMMITTED). +// that are in the ACTIVE phase. func activeSessionsOnCurrentHead() ([]*session.State, error) { repo, err := openRepository() if err != nil { diff --git a/cmd/entire/cli/session/phase.go b/cmd/entire/cli/session/phase.go index f89975849..7478dcff8 100644 --- a/cmd/entire/cli/session/phase.go +++ b/cmd/entire/cli/session/phase.go @@ -12,14 +12,21 @@ import ( type Phase string const ( - PhaseActive Phase = "active" - PhaseActiveCommitted Phase = "active_committed" - PhaseIdle Phase = "idle" - PhaseEnded Phase = "ended" + PhaseActive Phase = "active" + PhaseIdle Phase = "idle" + PhaseEnded Phase = "ended" ) +// PhaseActiveCommitted is kept as a variable (not a valid phase constant) so that +// external packages referencing it continue to compile during the migration. +// It maps to PhaseActive at runtime via PhaseFromString. +// +// Deprecated: ACTIVE_COMMITTED has been removed from the state machine. +// All references should be migrated to PhaseActive. +var PhaseActiveCommitted Phase = "active_committed" + // allPhases is the canonical list of phases for enumeration (e.g., diagram generation). -var allPhases = []Phase{PhaseIdle, PhaseActive, PhaseActiveCommitted, PhaseEnded} +var allPhases = []Phase{PhaseIdle, PhaseActive, PhaseEnded} // PhaseFromString normalizes a phase string, treating empty or unknown values // as PhaseIdle for backward compatibility with pre-state-machine session files. @@ -27,8 +34,8 @@ func PhaseFromString(s string) Phase { switch Phase(s) { case PhaseActive: return PhaseActive - case PhaseActiveCommitted: - return PhaseActiveCommitted + case "active_committed": // backward compat: old state files + return PhaseActive case PhaseIdle: return PhaseIdle case PhaseEnded: @@ -40,7 +47,7 @@ func PhaseFromString(s string) Phase { // IsActive reports whether the phase represents an active agent turn. func (p Phase) IsActive() bool { - return p == PhaseActive || p == PhaseActiveCommitted + return p == PhaseActive } // Event represents something that happened to a session. @@ -138,8 +145,6 @@ func Transition(current Phase, event Event, ctx TransitionContext) TransitionRes return transitionFromIdle(event, ctx) case PhaseActive: return transitionFromActive(event, ctx) - case PhaseActiveCommitted: - return transitionFromActiveCommitted(event, ctx) case PhaseEnded: return transitionFromEnded(event, ctx) default: @@ -183,10 +188,8 @@ func transitionFromIdle(event Event, ctx TransitionContext) TransitionResult { func transitionFromActive(event Event, ctx TransitionContext) TransitionResult { switch event { case EventTurnStart: - // Ctrl-C recovery: just continue. - // This is a degenerate case where the EndTurn is skipped after a in-turn commit. - // Either the agent crashed or the user interrupted it. - // We choose to continue, and defer condensation to the next TurnEnd or GitCommit. + // Ctrl-C recovery: the previous turn's TurnEnd was skipped (agent crashed or + // user interrupted). Just continue as active. return TransitionResult{ NewPhase: PhaseActive, Actions: []Action{ActionUpdateLastInteraction}, @@ -201,8 +204,8 @@ func transitionFromActive(event Event, ctx TransitionContext) TransitionResult { return TransitionResult{NewPhase: PhaseActive} } return TransitionResult{ - NewPhase: PhaseActiveCommitted, - Actions: []Action{ActionMigrateShadowBranch, ActionUpdateLastInteraction}, + NewPhase: PhaseActive, + Actions: []Action{ActionCondense, ActionMigrateShadowBranch, ActionUpdateLastInteraction}, } case EventSessionStart: return TransitionResult{ @@ -219,45 +222,6 @@ func transitionFromActive(event Event, ctx TransitionContext) TransitionResult { } } -func transitionFromActiveCommitted(event Event, ctx TransitionContext) TransitionResult { - switch event { - case EventTurnStart: - // Ctrl-C recovery after commit. - // This is a degenerate case where the EndTurn is skipped after a in-turn commit. - // Either the agent crashed or the user interrupted it. - // We choose to continue, and defer condensation to the next TurnEnd or GitCommit. - return TransitionResult{ - NewPhase: PhaseActive, - Actions: []Action{ActionUpdateLastInteraction}, - } - case EventTurnEnd: - return TransitionResult{ - NewPhase: PhaseIdle, - Actions: []Action{ActionCondense, ActionUpdateLastInteraction}, - } - case EventGitCommit: - if ctx.IsRebaseInProgress { - return TransitionResult{NewPhase: PhaseActiveCommitted} - } - return TransitionResult{ - NewPhase: PhaseActiveCommitted, - Actions: []Action{ActionMigrateShadowBranch, ActionUpdateLastInteraction}, - } - case EventSessionStart: - return TransitionResult{ - NewPhase: PhaseActiveCommitted, - Actions: []Action{ActionWarnStaleSession}, - } - case EventSessionStop: - return TransitionResult{ - NewPhase: PhaseEnded, - Actions: []Action{ActionUpdateLastInteraction}, - } - default: - return TransitionResult{NewPhase: PhaseActiveCommitted} - } -} - func transitionFromEnded(event Event, ctx TransitionContext) TransitionResult { switch event { case EventTurnStart: @@ -332,7 +296,6 @@ func MermaidDiagram() string { // State declarations with descriptions. b.WriteString(" state \"IDLE\" as idle\n") b.WriteString(" state \"ACTIVE\" as active\n") - b.WriteString(" state \"ACTIVE_COMMITTED\" as active_committed\n") b.WriteString(" state \"ENDED\" as ended\n") b.WriteString("\n") diff --git a/cmd/entire/cli/session/phase_test.go b/cmd/entire/cli/session/phase_test.go index 009b874e9..458a4d3ea 100644 --- a/cmd/entire/cli/session/phase_test.go +++ b/cmd/entire/cli/session/phase_test.go @@ -17,7 +17,7 @@ func TestPhaseFromString(t *testing.T) { want Phase }{ {name: "active", input: "active", want: PhaseActive}, - {name: "active_committed", input: "active_committed", want: PhaseActiveCommitted}, + {name: "active_committed_maps_to_active", input: "active_committed", want: PhaseActive}, {name: "idle", input: "idle", want: PhaseIdle}, {name: "ended", input: "ended", want: PhaseEnded}, {name: "empty_string_defaults_to_idle", input: "", want: PhaseIdle}, @@ -43,7 +43,6 @@ func TestPhase_IsActive(t *testing.T) { want bool }{ {name: "active_is_active", phase: PhaseActive, want: true}, - {name: "active_committed_is_active", phase: PhaseActiveCommitted, want: true}, {name: "idle_is_not_active", phase: PhaseIdle, want: false}, {name: "ended_is_not_active", phase: PhaseEnded, want: false}, } @@ -192,11 +191,11 @@ func TestTransitionFromActive(t *testing.T) { wantActions: []Action{ActionUpdateLastInteraction}, }, { - name: "GitCommit_transitions_to_ACTIVE_COMMITTED", + name: "GitCommit_condenses_immediately", current: PhaseActive, event: EventGitCommit, - wantPhase: PhaseActiveCommitted, - wantActions: []Action{ActionMigrateShadowBranch, ActionUpdateLastInteraction}, + wantPhase: PhaseActive, + wantActions: []Action{ActionCondense, ActionMigrateShadowBranch, ActionUpdateLastInteraction}, }, { name: "GitCommit_rebase_skips_everything", @@ -223,51 +222,33 @@ func TestTransitionFromActive(t *testing.T) { }) } -func TestTransitionFromActiveCommitted(t *testing.T) { +func TestPhaseFromString_ActiveCommittedMigration(t *testing.T) { + t.Parallel() + got := PhaseFromString("active_committed") + if got != PhaseActive { + t.Errorf("PhaseFromString(\"active_committed\") = %q, want %q", got, PhaseActive) + } +} + +func TestTransition_ActiveCommittedBackwardCompat(t *testing.T) { t.Parallel() + + // State files may still contain "active_committed" from before this phase + // was removed. Transition normalizes it to PhaseActive via PhaseFromString. runTransitionTests(t, []transitionCase{ { - name: "TurnEnd_transitions_to_IDLE_with_condense", - current: PhaseActiveCommitted, + name: "active_committed_TurnEnd_treated_as_ACTIVE", + current: Phase("active_committed"), event: EventTurnEnd, wantPhase: PhaseIdle, - wantActions: []Action{ActionCondense, ActionUpdateLastInteraction}, - }, - { - name: "GitCommit_stays_with_migrate", - current: PhaseActiveCommitted, - event: EventGitCommit, - wantPhase: PhaseActiveCommitted, - wantActions: []Action{ActionMigrateShadowBranch, ActionUpdateLastInteraction}, + wantActions: []Action{ActionUpdateLastInteraction}, }, { - name: "GitCommit_rebase_skips_everything", - current: PhaseActiveCommitted, + name: "active_committed_GitCommit_treated_as_ACTIVE", + current: Phase("active_committed"), event: EventGitCommit, - ctx: TransitionContext{IsRebaseInProgress: true}, - wantPhase: PhaseActiveCommitted, - wantActions: nil, - }, - { - name: "TurnStart_transitions_to_ACTIVE", - current: PhaseActiveCommitted, - event: EventTurnStart, wantPhase: PhaseActive, - wantActions: []Action{ActionUpdateLastInteraction}, - }, - { - name: "SessionStop_transitions_to_ENDED", - current: PhaseActiveCommitted, - event: EventSessionStop, - wantPhase: PhaseEnded, - wantActions: []Action{ActionUpdateLastInteraction}, - }, - { - name: "SessionStart_warns_stale_session", - current: PhaseActiveCommitted, - event: EventSessionStart, - wantPhase: PhaseActiveCommitted, - wantActions: []Action{ActionWarnStaleSession}, + wantActions: []Action{ActionCondense, ActionMigrateShadowBranch, ActionUpdateLastInteraction}, }, }) } @@ -468,7 +449,7 @@ func TestApplyCommonActions_ClearsEndedAt(t *testing.T) { func TestApplyCommonActions_PassesThroughStrategyActions(t *testing.T) { t.Parallel() - state := &State{Phase: PhaseActiveCommitted} + state := &State{Phase: PhaseActive} result := TransitionResult{ NewPhase: PhaseIdle, Actions: []Action{ActionCondense, ActionUpdateLastInteraction}, @@ -487,14 +468,14 @@ func TestApplyCommonActions_MultipleStrategyActions(t *testing.T) { state := &State{Phase: PhaseActive} result := TransitionResult{ - NewPhase: PhaseActiveCommitted, - Actions: []Action{ActionMigrateShadowBranch, ActionUpdateLastInteraction}, + NewPhase: PhaseActive, + Actions: []Action{ActionCondense, ActionMigrateShadowBranch, ActionUpdateLastInteraction}, } remaining := ApplyCommonActions(state, result) - assert.Equal(t, []Action{ActionMigrateShadowBranch}, remaining) - assert.Equal(t, PhaseActiveCommitted, state.Phase) + assert.Equal(t, []Action{ActionCondense, ActionMigrateShadowBranch}, remaining) + assert.Equal(t, PhaseActive, state.Phase) } func TestApplyCommonActions_WarnStaleSessionPassedThrough(t *testing.T) { @@ -572,13 +553,13 @@ func TestMermaidDiagram(t *testing.T) { assert.Contains(t, diagram, "stateDiagram-v2") assert.Contains(t, diagram, "IDLE") assert.Contains(t, diagram, "ACTIVE") - assert.Contains(t, diagram, "ACTIVE_COMMITTED") assert.Contains(t, diagram, "ENDED") + assert.NotContains(t, diagram, "ACTIVE_COMMITTED") // Verify key transitions are present. assert.Contains(t, diagram, "idle --> active") - assert.Contains(t, diagram, "active --> active_committed") - assert.Contains(t, diagram, "active_committed --> idle") + assert.Contains(t, diagram, "active --> active") // GitCommit stays ACTIVE now + assert.Contains(t, diagram, "active --> idle") // TurnEnd assert.Contains(t, diagram, "ended --> idle") assert.Contains(t, diagram, "ended --> active") diff --git a/cmd/entire/cli/session/state.go b/cmd/entire/cli/session/state.go index 950b84c10..b40e3c3d8 100644 --- a/cmd/entire/cli/session/state.go +++ b/cmd/entire/cli/session/state.go @@ -37,7 +37,7 @@ type State struct { // AttributionBaseCommit is the commit used as the reference point for attribution calculations. // Unlike BaseCommit (which tracks the shadow branch and moves with migration), this field - // preserves the original base commit so deferred condensation can correctly calculate + // preserves the original base commit so condensation can correctly calculate // agent vs human line attribution. Updated only after successful condensation. AttributionBaseCommit string `json:"attribution_base_commit,omitempty"` @@ -59,10 +59,6 @@ type State struct { // Empty means idle (backward compat with pre-state-machine files). Phase Phase `json:"phase,omitempty"` - // PendingCheckpointID is the checkpoint ID for the current commit cycle. - // Generated once when first needed, reused across all commits in the session. - PendingCheckpointID string `json:"pending_checkpoint_id,omitempty"` - // LastInteractionTime is updated on every hook invocation. // Used for stale session detection in "entire doctor". LastInteractionTime *time.Time `json:"last_interaction_time,omitempty"` diff --git a/cmd/entire/cli/strategy/manual_commit_condensation.go b/cmd/entire/cli/strategy/manual_commit_condensation.go index 6f7475694..386009a62 100644 --- a/cmd/entire/cli/strategy/manual_commit_condensation.go +++ b/cmd/entire/cli/strategy/manual_commit_condensation.go @@ -606,7 +606,6 @@ func (s *ManualCommitStrategy) CondenseSessionByID(sessionID string) error { state.CheckpointTranscriptStart = result.TotalTranscriptLines state.Phase = session.PhaseIdle state.LastCheckpointID = checkpointID - state.PendingCheckpointID = "" // Clear after condensation (amend handler uses LastCheckpointID) state.AttributionBaseCommit = state.BaseCommit state.PromptAttributions = nil state.PendingPromptAttribution = nil diff --git a/cmd/entire/cli/strategy/manual_commit_git.go b/cmd/entire/cli/strategy/manual_commit_git.go index cf4cb841e..d506f5478 100644 --- a/cmd/entire/cli/strategy/manual_commit_git.go +++ b/cmd/entire/cli/strategy/manual_commit_git.go @@ -115,10 +115,9 @@ func (s *ManualCommitStrategy) SaveChanges(ctx SaveContext) error { // Update session state state.StepCount++ - // Note: PendingCheckpointID is intentionally NOT cleared here. - // It is set by PostCommit (ACTIVE → ACTIVE_COMMITTED) and consumed by - // handleTurnEndCondense. Clearing it here would cause a mismatch between - // the checkpoint ID in the commit trailer and the condensed metadata. + // Note: LastCheckpointID is intentionally NOT cleared here. + // It is set by condenseAndUpdateState and should persist across + // SaveChanges calls for split-commit reuse. // Store the prompt attribution we calculated before saving state.PromptAttributions = append(state.PromptAttributions, promptAttr) diff --git a/cmd/entire/cli/strategy/manual_commit_hooks.go b/cmd/entire/cli/strategy/manual_commit_hooks.go index b5f927957..398b09736 100644 --- a/cmd/entire/cli/strategy/manual_commit_hooks.go +++ b/cmd/entire/cli/strategy/manual_commit_hooks.go @@ -210,7 +210,7 @@ func isGitSequenceOperation() bool { // - "" or "template": normal editor flow - adds trailer with explanatory comment // - "message": using -m or -F flag - prompts user interactively via /dev/tty // - "merge", "squash": skip trailer entirely (auto-generated messages) -// - "commit": amend operation - preserves existing trailer or restores from PendingCheckpointID +// - "commit": amend operation - preserves existing trailer or restores from LastCheckpointID // func (s *ManualCommitStrategy) PrepareCommitMsg(commitMsgFile string, source string) error { //nolint:maintidx // already present in codebase @@ -380,23 +380,12 @@ func (s *ManualCommitStrategy) PrepareCommitMsg(commitMsgFile string, source str } if hasNewContent { - // New content: check PendingCheckpointID first (set during previous condensation), - // otherwise generate a new one. This ensures idempotent IDs across hook invocations. - for _, state := range sessionsWithContent { - if state.PendingCheckpointID != "" { - if cpID, err := id.NewCheckpointID(state.PendingCheckpointID); err == nil { - checkpointID = cpID - break - } - } - } - if checkpointID.IsEmpty() { - cpID, err := id.Generate() - if err != nil { - return fmt.Errorf("failed to generate checkpoint ID: %w", err) - } - checkpointID = cpID + // Generate a fresh checkpoint ID for new content. + cpID, err := id.Generate() + if err != nil { + return fmt.Errorf("failed to generate checkpoint ID: %w", err) } + checkpointID = cpID } // Otherwise checkpointID is already set to LastCheckpointID from above @@ -424,18 +413,7 @@ func (s *ManualCommitStrategy) PrepareCommitMsg(commitMsgFile string, source str // vs linking a genuinely new checkpoint. Restoring doesn't need user confirmation // since the data is already committed — this handles git commit --amend -m "..." // and non-interactive environments (e.g., Claude doing commits). - isRestoringExisting := false - if !hasNewContent && reusedSession != nil { - // Reusing LastCheckpointID from a previous condensation - isRestoringExisting = true - } else if hasNewContent { - for _, state := range sessionsWithContent { - if state.PendingCheckpointID != "" { - isRestoringExisting = true - break - } - } - } + isRestoringExisting := !hasNewContent && reusedSession != nil // Add trailer differently based on commit source switch { @@ -483,7 +461,7 @@ func (s *ManualCommitStrategy) PrepareCommitMsg(commitMsgFile string, source str } // handleAmendCommitMsg handles the prepare-commit-msg hook for amend operations -// (source="commit"). It preserves existing trailers or restores from PendingCheckpointID. +// (source="commit"). It preserves existing trailers or restores from LastCheckpointID. func (s *ManualCommitStrategy) handleAmendCommitMsg(logCtx context.Context, commitMsgFile string) error { // Read current commit message content, err := os.ReadFile(commitMsgFile) //nolint:gosec // commitMsgFile is provided by git hook @@ -502,7 +480,7 @@ func (s *ManualCommitStrategy) handleAmendCommitMsg(logCtx context.Context, comm return nil } - // No trailer in message — check if any session has PendingCheckpointID to restore + // No trailer in message — check if any session has LastCheckpointID to restore worktreePath, err := GetWorktreePath() if err != nil { return nil //nolint:nilerr // Hook must be silent on failure @@ -527,41 +505,26 @@ func (s *ManualCommitStrategy) handleAmendCommitMsg(logCtx context.Context, comm } currentHead := head.Hash().String() - // Find first matching session with PendingCheckpointID or LastCheckpointID to restore. - // PendingCheckpointID is set during ACTIVE_COMMITTED (deferred condensation). + // Find first matching session with LastCheckpointID to restore. // LastCheckpointID is set after condensation completes. for _, state := range sessions { if state.BaseCommit != currentHead { continue } - var cpID id.CheckpointID - source := "" - - if state.PendingCheckpointID != "" { - if parsed, parseErr := id.NewCheckpointID(state.PendingCheckpointID); parseErr == nil { - cpID = parsed - source = "PendingCheckpointID" - } - } - if cpID.IsEmpty() && !state.LastCheckpointID.IsEmpty() { - cpID = state.LastCheckpointID - source = "LastCheckpointID" - } - if cpID.IsEmpty() { + if state.LastCheckpointID.IsEmpty() { continue } // Restore the trailer - message = addCheckpointTrailer(message, cpID) + message = addCheckpointTrailer(message, state.LastCheckpointID) if writeErr := os.WriteFile(commitMsgFile, []byte(message), 0o600); writeErr != nil { return nil //nolint:nilerr // Hook must be silent on failure } logging.Info(logCtx, "prepare-commit-msg: restored trailer on amend", slog.String("strategy", "manual-commit"), - slog.String("checkpoint_id", cpID.String()), + slog.String("checkpoint_id", state.LastCheckpointID.String()), slog.String("session_id", state.SessionID), - slog.String("source", source), ) return nil } @@ -575,14 +538,16 @@ func (s *ManualCommitStrategy) handleAmendCommitMsg(logCtx context.Context, comm // PostCommit is called by the git post-commit hook after a commit is created. // Uses the session state machine to determine what action to take per session: -// - ACTIVE → ACTIVE_COMMITTED: defer condensation (agent still working) +// - ACTIVE → condense immediately, migrate shadow branch, stay ACTIVE // - IDLE → condense immediately -// - ACTIVE_COMMITTED → migrate shadow branch (additional commit during same turn) // - ENDED → condense if files touched, discard if empty // // Shadow branches are only deleted when ALL sessions sharing the branch are non-active. // During rebase/cherry-pick/revert operations, phase transitions are skipped entirely. // +// Actions are processed in order: condense BEFORE migrate, so condensation reads +// from the old shadow branch before it gets renamed. +// //nolint:unparam // error return required by interface but hooks must return nil func (s *ManualCommitStrategy) PostCommit() error { logCtx := logging.WithComponent(context.Background(), "checkpoint") @@ -646,16 +611,6 @@ func (s *ManualCommitStrategy) PostCommit() error { newHead := head.Hash().String() - // Two-pass processing: condensation first, migration second. - // This prevents a migration from renaming a shadow branch before another - // session sharing that branch has had a chance to condense from it. - type pendingMigration struct { - state *SessionState - } - var pendingMigrations []pendingMigration - - // Pass 1: Run transitions and dispatch condensation/discard actions. - // Defer migration actions to pass 2. for _, state := range sessions { shadowBranchName := getShadowBranchNameForCommit(state.BaseCommit, state.WorktreeID) @@ -675,7 +630,11 @@ func (s *ManualCommitStrategy) PostCommit() error { // Run the state machine transition remaining := TransitionAndLog(state, session.EventGitCommit, transitionCtx) - // Dispatch strategy-specific actions. + // Track whether we need to migrate after condensation + needsMigration := false + + // Dispatch strategy-specific actions in order. + // Condense BEFORE migrate so condensation reads from the old shadow branch. // Each branch handles its own BaseCommit update so there is no // fallthrough conditional at the end. On condensation failure, // BaseCommit is intentionally NOT updated to preserve access to @@ -709,11 +668,8 @@ func (s *ManualCommitStrategy) PostCommit() error { } s.updateBaseCommitIfChanged(logCtx, state, newHead) case session.ActionMigrateShadowBranch: - // Deferred to pass 2 so condensation reads the old shadow branch first. - // Migration updates BaseCommit as part of the rename. - // Store checkpointID so HandleTurnEnd can reuse it for deferred condensation. - state.PendingCheckpointID = checkpointID.String() - pendingMigrations = append(pendingMigrations, pendingMigration{state: state}) + // Deferred until after condensation actions are processed. + needsMigration = true case session.ActionClearEndedAt, session.ActionUpdateLastInteraction: // Handled by session.ApplyCommonActions above case session.ActionWarnStaleSession: @@ -721,31 +677,31 @@ func (s *ManualCommitStrategy) PostCommit() error { } } + // Run migration after all condensation actions for this session. + if needsMigration { + if _, migErr := s.migrateShadowBranchIfNeeded(repo, state); migErr != nil { + logging.Warn(logCtx, "post-commit: shadow branch migration failed", + slog.String("session_id", state.SessionID), + slog.String("error", migErr.Error()), + ) + } + } + // Save the updated state if err := s.saveSessionState(state); err != nil { fmt.Fprintf(os.Stderr, "[entire] Warning: failed to update session state: %v\n", err) } - // Track whether any session on this shadow branch is still active - if state.Phase.IsActive() { + // Track whether any session on this shadow branch is still active. + // After condensation + migration, BaseCommit is updated to newHead, + // so the session moves to a new shadow branch. Only mark the OLD + // branch as having an active session if the session is still on it. + currentShadowBranch := getShadowBranchNameForCommit(state.BaseCommit, state.WorktreeID) + if state.Phase.IsActive() && currentShadowBranch == shadowBranchName { activeSessionsOnBranch[shadowBranchName] = true } } - // Pass 2: Run deferred migrations now that all condensations are complete. - for _, pm := range pendingMigrations { - if _, migErr := s.migrateShadowBranchIfNeeded(repo, pm.state); migErr != nil { - logging.Warn(logCtx, "post-commit: shadow branch migration failed", - slog.String("session_id", pm.state.SessionID), - slog.String("error", migErr.Error()), - ) - } - // Save the migrated state - if err := s.saveSessionState(pm.state); err != nil { - fmt.Fprintf(os.Stderr, "[entire] Warning: failed to update session state after migration: %v\n", err) - } - } - // Clean up shadow branches — only delete when ALL sessions on the branch are non-active for shadowBranchName := range shadowBranchesToDelete { if activeSessionsOnBranch[shadowBranchName] { @@ -807,10 +763,6 @@ func (s *ManualCommitStrategy) condenseAndUpdateState( // Save checkpoint ID so subsequent commits can reuse it state.LastCheckpointID = checkpointID - // Clear PendingCheckpointID after condensation — it was used for deferred - // condensation (ACTIVE_COMMITTED flow) and should not persist. The amend - // handler uses LastCheckpointID instead. - state.PendingCheckpointID = "" shortID := state.SessionID if len(shortID) > 8 { @@ -1068,21 +1020,10 @@ func (s *ManualCommitStrategy) sessionHasNewContentFromLiveTranscript(repo *git. // (ACTIVE session + no TTY). Generates a checkpoint ID and adds the trailer // directly, bypassing content detection and interactive prompts. func (s *ManualCommitStrategy) addTrailerForAgentCommit(logCtx context.Context, commitMsgFile string, state *SessionState, source string) error { - // Use PendingCheckpointID if set, otherwise generate a new one - var cpID id.CheckpointID - if state.PendingCheckpointID != "" { - var err error - cpID, err = id.NewCheckpointID(state.PendingCheckpointID) - if err != nil { - cpID = "" // fall through to generate - } - } - if cpID.IsEmpty() { - var err error - cpID, err = id.Generate() - if err != nil { - return nil //nolint:nilerr // Hook must be silent on failure - } + // Always generate a fresh checkpoint ID for agent commits + cpID, err := id.Generate() + if err != nil { + return nil //nolint:nilerr // Hook must be silent on failure } content, err := os.ReadFile(commitMsgFile) //nolint:gosec // commitMsgFile is provided by git hook @@ -1239,16 +1180,25 @@ func (s *ManualCommitStrategy) InitializeSession(sessionID string, agentType age state.TranscriptPath = transcriptPath } - // Clear checkpoint IDs on every new prompt - // These are set during PostCommit when a checkpoint is created, and should be - // cleared when the user enters a new prompt (starting fresh work) - if state.LastCheckpointID != "" { - state.LastCheckpointID = "" - } - if state.PendingCheckpointID != "" { - state.PendingCheckpointID = "" + // Append any trailing transcript from the previous turn to the committed + // checkpoint before clearing LastCheckpointID. This captures conversation + // that happened after the commit (e.g., agent explaining what it did) but + // before the user entered a new prompt. + if !state.LastCheckpointID.IsEmpty() { + if err := s.handleTrailingTranscript(state); err != nil { + logCtx := logging.WithComponent(context.Background(), "trailing-transcript") + logging.Warn(logCtx, "trailing transcript append failed during InitializeSession", + slog.String("session_id", state.SessionID), + slog.String("error", err.Error()), + ) + } } + // Clear checkpoint ID on every new prompt + // This is set during PostCommit when a checkpoint is created, and should be + // cleared when the user enters a new prompt (starting fresh work) + state.LastCheckpointID = "" + // Calculate attribution at prompt start (BEFORE agent makes any changes) // This captures user edits since the last checkpoint (or base commit for first prompt). // IMPORTANT: Always calculate attribution, even for the first checkpoint, to capture @@ -1449,22 +1399,30 @@ func (s *ManualCommitStrategy) getLastPrompt(repo *git.Repository, state *Sessio } // HandleTurnEnd dispatches strategy-specific actions emitted when an agent turn ends. -// This handles the ACTIVE_COMMITTED → IDLE transition where ActionCondense is deferred -// from PostCommit (agent was still active during the commit). +// Since condensation now happens immediately in PostCommit, HandleTurnEnd handles: +// - Trailing transcript: appends post-commit conversation to the prior checkpoint +// when no new files were touched (StepCount == 0). +// - Unexpected action logging for diagnostics. // //nolint:unparam // error return required by interface but hooks must return nil func (s *ManualCommitStrategy) HandleTurnEnd(state *SessionState, actions []session.Action) error { + logCtx := logging.WithComponent(context.Background(), "checkpoint") + + // Attempt to append trailing transcript (best-effort) + if err := s.handleTrailingTranscript(state); err != nil { + logging.Warn(logCtx, "trailing transcript append failed", + slog.String("session_id", state.SessionID), + slog.String("error", err.Error()), + ) + } + if len(actions) == 0 { return nil } - logCtx := logging.WithComponent(context.Background(), "checkpoint") - for _, action := range actions { switch action { - case session.ActionCondense: - s.handleTurnEndCondense(logCtx, state) - case session.ActionCondenseIfFilesTouched, session.ActionDiscardIfNoFiles, + case session.ActionCondense, session.ActionCondenseIfFilesTouched, session.ActionDiscardIfNoFiles, session.ActionMigrateShadowBranch, session.ActionWarnStaleSession: // Not expected at turn-end; log for diagnostics. logging.Debug(logCtx, "turn-end: unexpected action", @@ -1478,100 +1436,6 @@ func (s *ManualCommitStrategy) HandleTurnEnd(state *SessionState, actions []sess return nil } -// handleTurnEndCondense performs deferred condensation at turn end. -func (s *ManualCommitStrategy) handleTurnEndCondense(logCtx context.Context, state *SessionState) { - repo, err := OpenRepository() - if err != nil { - logging.Warn(logCtx, "turn-end condense: failed to open repo", - slog.String("error", err.Error())) - return - } - - head, err := repo.Head() - if err != nil { - logging.Warn(logCtx, "turn-end condense: failed to get HEAD", - slog.String("error", err.Error())) - return - } - - // Derive checkpoint ID from PendingCheckpointID (set during PostCommit), - // or generate a new one if not set. - var checkpointID id.CheckpointID - if state.PendingCheckpointID != "" { - if cpID, parseErr := id.NewCheckpointID(state.PendingCheckpointID); parseErr == nil { - checkpointID = cpID - } - } - if checkpointID.IsEmpty() { - cpID, genErr := id.Generate() - if genErr != nil { - logging.Warn(logCtx, "turn-end condense: failed to generate checkpoint ID", - slog.String("error", genErr.Error())) - return - } - checkpointID = cpID - } - - // Check if there is actually new content to condense. - // Fail-open: if content check errors, assume new content so we don't silently skip. - hasNew, contentErr := s.sessionHasNewContent(repo, state) - if contentErr != nil { - hasNew = true - logging.Debug(logCtx, "turn-end condense: error checking content, assuming new", - slog.String("session_id", state.SessionID), - slog.String("error", contentErr.Error())) - } - - if !hasNew { - logging.Debug(logCtx, "turn-end condense: no new content", - slog.String("session_id", state.SessionID)) - return - } - - shadowBranchName := getShadowBranchNameForCommit(state.BaseCommit, state.WorktreeID) - shadowBranchesToDelete := map[string]struct{}{} - - s.condenseAndUpdateState(logCtx, repo, checkpointID, state, head, shadowBranchName, shadowBranchesToDelete) - - // Delete shadow branches after condensation — but only if no other active - // sessions share the branch (same safety check PostCommit uses). - for branchName := range shadowBranchesToDelete { - if s.hasOtherActiveSessionsOnBranch(state.SessionID, state.BaseCommit, state.WorktreeID) { - logging.Debug(logCtx, "turn-end: preserving shadow branch (other active session exists)", - slog.String("shadow_branch", branchName)) - continue - } - if err := deleteShadowBranch(repo, branchName); err != nil { - fmt.Fprintf(os.Stderr, "[entire] Warning: failed to clean up %s: %v\n", branchName, err) - } else { - fmt.Fprintf(os.Stderr, "[entire] Cleaned up shadow branch: %s\n", branchName) - logging.Info(logCtx, "shadow branch deleted (turn-end)", - slog.String("strategy", "manual-commit"), - slog.String("shadow_branch", branchName), - ) - } - } -} - -// hasOtherActiveSessionsOnBranch checks if any other sessions with the same -// base commit and worktree ID are in an active phase. Used to prevent deleting -// a shadow branch that another session still needs. -func (s *ManualCommitStrategy) hasOtherActiveSessionsOnBranch(currentSessionID, baseCommit, worktreeID string) bool { - sessions, err := s.findSessionsForCommit(baseCommit) - if err != nil { - return false // Fail-open: if we can't check, don't block deletion - } - for _, other := range sessions { - if other.SessionID == currentSessionID { - continue - } - if other.WorktreeID == worktreeID && other.Phase.IsActive() { - return true - } - } - return false -} - // getCondensedFilesTouched reads the files_touched list from the last condensed // checkpoint metadata on entire/checkpoints/v1. Used to check staged-file overlap // after FilesTouched has been reset by condensation. diff --git a/cmd/entire/cli/strategy/manual_commit_test.go b/cmd/entire/cli/strategy/manual_commit_test.go index 07aec19c6..8ecc718f2 100644 --- a/cmd/entire/cli/strategy/manual_commit_test.go +++ b/cmd/entire/cli/strategy/manual_commit_test.go @@ -1479,7 +1479,6 @@ func TestShadowStrategy_CondenseSession_EphemeralBranchTrailer(t *testing.T) { t.Fatalf("failed to create metadata dir: %v", err) } - //nolint:goconst // test data repeated across test functions transcript := `{"type":"human","message":{"content":"test prompt"}} {"type":"assistant","message":{"content":"test response"}} ` @@ -2456,7 +2455,7 @@ func TestMultiCheckpoint_UserEditsBetweenCheckpoints(t *testing.T) { // TestCondenseSession_PrefersLiveTranscript verifies that CondenseSession reads the // live transcript file when available, rather than the potentially stale shadow branch copy. // This reproduces the bug where SaveChanges was skipped (no code changes) but the -// transcript continued growing — deferred condensation would read stale data. +// transcript continued growing — condensation from the shadow branch would read stale data. func TestCondenseSession_PrefersLiveTranscript(t *testing.T) { dir := t.TempDir() repo, err := git.PlainInit(dir, false) diff --git a/cmd/entire/cli/strategy/manual_commit_trailing.go b/cmd/entire/cli/strategy/manual_commit_trailing.go new file mode 100644 index 000000000..cdd1a1dbc --- /dev/null +++ b/cmd/entire/cli/strategy/manual_commit_trailing.go @@ -0,0 +1,126 @@ +package strategy + +import ( + "context" + "fmt" + "log/slog" + "os" + + "github.com/entireio/cli/cmd/entire/cli/agent" + "github.com/entireio/cli/cmd/entire/cli/checkpoint" + "github.com/entireio/cli/cmd/entire/cli/logging" + "github.com/entireio/cli/cmd/entire/cli/transcript" +) + +// handleTrailingTranscript appends post-commit conversation to the prior checkpoint. +// Called from HandleTurnEnd (after SaveChanges) and InitializeSession (on new prompt). +// The call is idempotent: CheckpointTranscriptStart is advanced after each append, +// preventing double-appends across the two call sites. Only appends if: +// - LastCheckpointID is set (a condensation happened this session) +// - StepCount == 0 (no new files touched since condensation, meaning SaveChanges +// did not create a new checkpoint after condensation) +// - The live transcript has grown beyond CheckpointTranscriptStart +// +// The trailing transcript (explanation, summary, etc.) is appended to the existing +// checkpoint on entire/checkpoints/v1 without modifying FilesTouched or TokenUsage. +func (s *ManualCommitStrategy) handleTrailingTranscript(state *SessionState) error { + logCtx := logging.WithComponent(context.Background(), "trailing-transcript") + + // Guard: no prior condensation + if state.LastCheckpointID.IsEmpty() { + return nil + } + + // Guard: SaveChanges created a new checkpoint (new files touched) + if state.StepCount > 0 { + return nil + } + + // Guard: transcript path must be available + if state.TranscriptPath == "" { + logging.Debug(logCtx, "trailing transcript: no transcript path", + slog.String("session_id", state.SessionID), + ) + return nil + } + + // Read the live transcript to check length + transcriptData, err := os.ReadFile(state.TranscriptPath) + if err != nil { + logging.Debug(logCtx, "trailing transcript: failed to read transcript", + slog.String("session_id", state.SessionID), + slog.String("error", err.Error()), + ) + return nil // Best-effort: don't fail turn-end for this + } + + transcriptContent := string(transcriptData) + currentLines := countTranscriptItems(state.AgentType, transcriptContent) + + // Guard: no new content since last condensation + if currentLines <= state.CheckpointTranscriptStart { + logging.Debug(logCtx, "trailing transcript: no new content", + slog.String("session_id", state.SessionID), + slog.Int("current_lines", currentLines), + slog.Int("checkpoint_start", state.CheckpointTranscriptStart), + ) + return nil + } + + logging.Info(logCtx, "appending trailing transcript to checkpoint", + slog.String("session_id", state.SessionID), + slog.String("checkpoint_id", state.LastCheckpointID.String()), + slog.Int("trailing_lines", currentLines-state.CheckpointTranscriptStart), + ) + + // Extract trailing transcript (from CheckpointTranscriptStart to end) + var trailingTranscript []byte + if state.AgentType == agent.AgentTypeGemini { + // For Gemini, the transcript is a single JSON blob. We can't slice it + // by line. For now, skip trailing transcript for Gemini. + logging.Debug(logCtx, "trailing transcript: Gemini transcript slicing not supported", + slog.String("session_id", state.SessionID), + ) + return nil + } + trailingTranscript = transcript.SliceFromLine(transcriptData, state.CheckpointTranscriptStart) + if len(trailingTranscript) == 0 { + return nil + } + + // Extract prompts from trailing portion + trailingPrompts := extractUserPrompts(state.AgentType, string(trailingTranscript)) + + // Generate updated context from all prompts + // Read existing prompts and combine with trailing ones + allPrompts := extractUserPrompts(state.AgentType, transcriptContent) + updatedContext := generateContextFromPrompts(allPrompts) + + // Get checkpoint store + store, err := s.getCheckpointStore() + if err != nil { + return fmt.Errorf("failed to get checkpoint store: %w", err) + } + + // Append to existing checkpoint + if err := store.UpdateCommitted(context.Background(), checkpoint.UpdateCommittedOptions{ + CheckpointID: state.LastCheckpointID, + SessionID: state.SessionID, + Transcript: trailingTranscript, + Prompts: trailingPrompts, + Context: updatedContext, + }); err != nil { + return fmt.Errorf("failed to append trailing transcript: %w", err) + } + + // Update state to reflect the new transcript position + state.CheckpointTranscriptStart = currentLines + + logging.Info(logCtx, "trailing transcript appended successfully", + slog.String("session_id", state.SessionID), + slog.String("checkpoint_id", state.LastCheckpointID.String()), + slog.Int("new_transcript_start", currentLines), + ) + + return nil +} diff --git a/cmd/entire/cli/strategy/manual_commit_trailing_test.go b/cmd/entire/cli/strategy/manual_commit_trailing_test.go new file mode 100644 index 000000000..9d800835d --- /dev/null +++ b/cmd/entire/cli/strategy/manual_commit_trailing_test.go @@ -0,0 +1,193 @@ +package strategy + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/entireio/cli/cmd/entire/cli/agent" + "github.com/entireio/cli/cmd/entire/cli/checkpoint" + "github.com/entireio/cli/cmd/entire/cli/checkpoint/id" + "github.com/entireio/cli/cmd/entire/cli/session" + + "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing/object" +) + +func TestTrailingTranscript_NoLastCheckpointID_Skips(t *testing.T) { + t.Parallel() + + state := &SessionState{ + SessionID: "test-session", + StepCount: 0, + Phase: session.PhaseIdle, + AgentType: agent.AgentTypeClaudeCode, + // LastCheckpointID intentionally empty + } + + s := &ManualCommitStrategy{} + err := s.handleTrailingTranscript(state) + if err != nil { + t.Fatalf("handleTrailingTranscript() error = %v", err) + } + // Should be a no-op - nothing to verify except no error +} + +func TestTrailingTranscript_NewFilesTouched_Skips(t *testing.T) { + t.Parallel() + + state := &SessionState{ + SessionID: "test-session", + StepCount: 1, // SaveChanges created a checkpoint + LastCheckpointID: id.MustCheckpointID("a1b2c3d4e5f6"), + Phase: session.PhaseIdle, + AgentType: agent.AgentTypeClaudeCode, + } + + s := &ManualCommitStrategy{} + err := s.handleTrailingTranscript(state) + if err != nil { + t.Fatalf("handleTrailingTranscript() error = %v", err) + } + // Should be a no-op since StepCount > 0 (new files touched) +} + +func TestTrailingTranscript_NoNewTranscript_Skips(t *testing.T) { + t.Parallel() + + // Create a temporary transcript file with known content + tmpDir := t.TempDir() + transcriptPath := filepath.Join(tmpDir, "transcript.jsonl") + transcriptContent := `{"type":"human","message":{"content":"hello"}} +{"type":"assistant","message":{"content":"world"}} +` + if err := os.WriteFile(transcriptPath, []byte(transcriptContent), 0o600); err != nil { + t.Fatalf("failed to write transcript: %v", err) + } + + state := &SessionState{ + SessionID: "test-session", + StepCount: 0, + LastCheckpointID: id.MustCheckpointID("a1b2c3d4e5f6"), + Phase: session.PhaseIdle, + AgentType: agent.AgentTypeClaudeCode, + TranscriptPath: transcriptPath, + CheckpointTranscriptStart: 2, // Already condensed all 2 lines + } + + s := &ManualCommitStrategy{} + err := s.handleTrailingTranscript(state) + if err != nil { + t.Fatalf("handleTrailingTranscript() error = %v", err) + } + // Should be a no-op since transcript hasn't grown +} + +func TestTrailingTranscript_NoNewFiles_AppendsToCheckpoint(t *testing.T) { + t.Parallel() + + // Create a git repo with a committed checkpoint + tmpDir := t.TempDir() + repo, err := git.PlainInit(tmpDir, false) + if err != nil { + t.Fatalf("failed to init git repo: %v", err) + } + + // Create initial commit + worktree, err := repo.Worktree() + if err != nil { + t.Fatalf("failed to get worktree: %v", err) + } + readmeFile := filepath.Join(tmpDir, "README.md") + if err := os.WriteFile(readmeFile, []byte("# Test"), 0o644); err != nil { + t.Fatalf("failed to write README: %v", err) + } + if _, err := worktree.Add("README.md"); err != nil { + t.Fatalf("failed to add README: %v", err) + } + if _, err := worktree.Commit("Initial commit", &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@test.com"}, + }); err != nil { + t.Fatalf("failed to commit: %v", err) + } + + store := checkpoint.NewGitStore(repo) + checkpointID := id.MustCheckpointID("a1b2c3d4e5f6") + sessionID := "test-session-trailing" + + // Write initial checkpoint + initialTranscript := `{"type":"human","message":{"content":"initial prompt"}} +{"type":"assistant","message":{"content":"initial response"}}` + + err = store.WriteCommitted(context.Background(), checkpoint.WriteCommittedOptions{ + CheckpointID: checkpointID, + SessionID: sessionID, + Strategy: "manual-commit", + Transcript: []byte(initialTranscript), + Prompts: []string{"initial prompt"}, + Context: []byte("# Initial Context"), + FilesTouched: []string{"file1.go"}, + AuthorName: "Test", + AuthorEmail: "test@test.com", + }) + if err != nil { + t.Fatalf("WriteCommitted() error = %v", err) + } + + // Create transcript file with trailing content (3 lines total, 2 condensed) + transcriptPath := filepath.Join(tmpDir, "transcript.jsonl") + fullTranscript := `{"type":"human","message":{"content":"initial prompt"}} +{"type":"assistant","message":{"content":"initial response"}} +{"type":"human","message":{"content":"trailing question"}} +{"type":"assistant","message":{"content":"trailing answer"}} +` + if err := os.WriteFile(transcriptPath, []byte(fullTranscript), 0o600); err != nil { + t.Fatalf("failed to write transcript: %v", err) + } + + state := &SessionState{ + SessionID: sessionID, + StepCount: 0, + LastCheckpointID: checkpointID, + Phase: session.PhaseIdle, + AgentType: agent.AgentTypeClaudeCode, + TranscriptPath: transcriptPath, + CheckpointTranscriptStart: 2, // 2 lines already condensed + } + + // Create strategy with injected store. + // Pre-trigger the sync.Once by calling Do before it runs OpenRepository. + s := &ManualCommitStrategy{} + s.checkpointStoreOnce.Do(func() { + s.checkpointStore = store + }) + + err = s.handleTrailingTranscript(state) + if err != nil { + t.Fatalf("handleTrailingTranscript() error = %v", err) + } + + // Verify trailing content was appended + content, err := store.ReadSessionContentByID(context.Background(), checkpointID, sessionID) + if err != nil { + t.Fatalf("ReadSessionContentByID() error = %v", err) + } + + // Transcript should contain the trailing lines appended + if len(content.Transcript) <= len([]byte(initialTranscript)) { + t.Errorf("transcript length (%d) should be greater than initial (%d)", + len(content.Transcript), len(initialTranscript)) + } + + // Prompts should include trailing prompts + if content.Prompts == "initial prompt" { + t.Error("prompts should include trailing prompts, but only has initial") + } + + // CheckpointTranscriptStart should be updated to include trailing lines + if state.CheckpointTranscriptStart <= 2 { + t.Errorf("CheckpointTranscriptStart = %d, want > 2 (should be updated to include trailing lines)", + state.CheckpointTranscriptStart) + } +} diff --git a/cmd/entire/cli/strategy/mid_turn_commit_test.go b/cmd/entire/cli/strategy/mid_turn_commit_test.go index 7f24b854d..1b8eb2d1c 100644 --- a/cmd/entire/cli/strategy/mid_turn_commit_test.go +++ b/cmd/entire/cli/strategy/mid_turn_commit_test.go @@ -155,63 +155,3 @@ func TestPostCommit_NoTrailer_UpdatesBaseCommit(t *testing.T) { assert.Equal(t, session.PhaseActive, state.Phase, "Phase should remain ACTIVE when commit has no trailer") } - -// TestSaveChanges_PreservesPendingCheckpointID verifies that SaveChanges does NOT -// clear PendingCheckpointID. This field is set by PostCommit for deferred condensation -// and should persist through SaveChanges calls until consumed by handleTurnEndCondense. -// -// Bug: SaveChanges clears PendingCheckpointID at line ~120. When the agent stops, -// handleTurnEndCondense finds it empty, generates a new ID, and the commit trailer -// and condensed data end up with different IDs. -func TestSaveChanges_PreservesPendingCheckpointID(t *testing.T) { - dir := setupGitRepo(t) - t.Chdir(dir) - - repo, err := git.PlainOpen(dir) - require.NoError(t, err) - - s := &ManualCommitStrategy{} - sessionID := "test-preserve-pending-cpid" - - // Initialize session and save first checkpoint - setupSessionWithCheckpoint(t, s, repo, dir, sessionID) - - // Set PendingCheckpointID (simulating what PostCommit does) - state, err := s.loadSessionState(sessionID) - require.NoError(t, err) - const testPendingCpID = "abc123def456" - state.PendingCheckpointID = testPendingCpID - state.Phase = session.PhaseActiveCommitted - require.NoError(t, s.saveSessionState(state)) - - // Create metadata for a new checkpoint - metadataDir := ".entire/metadata/" + sessionID - metadataDirAbs := filepath.Join(dir, metadataDir) - // Transcript already exists from setupSessionWithCheckpoint - - // Modify a file so the checkpoint has real changes - testFile := filepath.Join(dir, "src", "new_file.go") - require.NoError(t, os.MkdirAll(filepath.Dir(testFile), 0o755)) - require.NoError(t, os.WriteFile(testFile, []byte("package src\n"), 0o644)) - - // Call SaveChanges — this should NOT clear PendingCheckpointID - err = s.SaveChanges(SaveContext{ - SessionID: sessionID, - ModifiedFiles: []string{}, - NewFiles: []string{"src/new_file.go"}, - DeletedFiles: []string{}, - MetadataDir: metadataDir, - MetadataDirAbs: metadataDirAbs, - CommitMessage: "Checkpoint 2", - AuthorName: "Test", - AuthorEmail: "test@test.com", - }) - require.NoError(t, err) - - // Reload state and verify PendingCheckpointID is preserved - state, err = s.loadSessionState(sessionID) - require.NoError(t, err) - assert.Equal(t, testPendingCpID, state.PendingCheckpointID, - "PendingCheckpointID should be preserved across SaveChanges calls, "+ - "not cleared — it's needed for deferred condensation at turn end") -} diff --git a/cmd/entire/cli/strategy/phase_postcommit_test.go b/cmd/entire/cli/strategy/phase_postcommit_test.go index 0d1115908..d153e69fb 100644 --- a/cmd/entire/cli/strategy/phase_postcommit_test.go +++ b/cmd/entire/cli/strategy/phase_postcommit_test.go @@ -20,10 +20,10 @@ import ( "github.com/stretchr/testify/require" ) -// TestPostCommit_ActiveSession_NoCondensation verifies that PostCommit on an -// ACTIVE session transitions to ACTIVE_COMMITTED without condensing. -// The shadow branch must be preserved because the session is still active. -func TestPostCommit_ActiveSession_NoCondensation(t *testing.T) { +// TestPostCommit_ActiveSession_CondensesImmediately verifies that PostCommit on an +// ACTIVE session condenses immediately and stays ACTIVE. The shadow branch is +// deleted since no other sessions need it. +func TestPostCommit_ActiveSession_CondensesImmediately(t *testing.T) { dir := setupGitRepo(t) t.Chdir(dir) @@ -42,6 +42,9 @@ func TestPostCommit_ActiveSession_NoCondensation(t *testing.T) { state.Phase = session.PhaseActive require.NoError(t, s.saveSessionState(state)) + // Record shadow branch name before PostCommit + shadowBranch := getShadowBranchNameForCommit(state.BaseCommit, state.WorktreeID) + // Create a commit WITH the Entire-Checkpoint trailer on the main branch commitWithCheckpointTrailer(t, repo, dir, "a1b2c3d4e5f6") @@ -49,20 +52,27 @@ func TestPostCommit_ActiveSession_NoCondensation(t *testing.T) { err = s.PostCommit() require.NoError(t, err) - // Verify phase transitioned to ACTIVE_COMMITTED + // Verify phase stays ACTIVE (no longer transitions to ACTIVE_COMMITTED) state, err = s.loadSessionState(sessionID) require.NoError(t, err) require.NotNil(t, state) - assert.Equal(t, session.PhaseActiveCommitted, state.Phase, - "ACTIVE session should transition to ACTIVE_COMMITTED on GitCommit") + assert.Equal(t, session.PhaseActive, state.Phase, + "ACTIVE session should stay ACTIVE on GitCommit (condenses immediately)") - // Verify shadow branch is NOT deleted (session is still active). - // After PostCommit, BaseCommit is updated to new HEAD, so use the current state. - shadowBranch := getShadowBranchNameForCommit(state.BaseCommit, state.WorktreeID) + // Verify condensation happened: the entire/checkpoints/v1 branch should exist + sessionsRef, err := repo.Reference(plumbing.NewBranchReferenceName(paths.MetadataBranchName), true) + require.NoError(t, err, "entire/checkpoints/v1 branch should exist after condensation") + assert.NotNil(t, sessionsRef) + + // Verify shadow branch IS deleted after condensation (only session on this branch) refName := plumbing.NewBranchReferenceName(shadowBranch) _, err = repo.Reference(refName, true) - assert.NoError(t, err, - "shadow branch should be preserved when session is still active") + require.Error(t, err, + "shadow branch should be deleted after condensation when no other sessions need it") + + // Verify StepCount was reset by condensation + assert.Equal(t, 0, state.StepCount, + "StepCount should be reset after condensation") } // TestPostCommit_IdleSession_Condenses verifies that PostCommit on an IDLE @@ -170,10 +180,10 @@ func TestPostCommit_RebaseDuringActive_SkipsTransition(t *testing.T) { "shadow branch should be preserved during rebase") } -// TestPostCommit_ShadowBranch_PreservedWhenActiveSessionExists verifies that -// the shadow branch is preserved when ANY session on it is still active, -// even if another session on the same branch is IDLE and gets condensed. -func TestPostCommit_ShadowBranch_PreservedWhenActiveSessionExists(t *testing.T) { +// TestPostCommit_ShadowBranch_PreservedWhenOtherSessionExists verifies that +// the shadow branch is preserved when another session that hasn't been +// condensed yet shares it, even after one session condenses. +func TestPostCommit_ShadowBranch_PreservedWhenOtherSessionExists(t *testing.T) { dir := setupGitRepo(t) t.Chdir(dir) @@ -221,30 +231,22 @@ func TestPostCommit_ShadowBranch_PreservedWhenActiveSessionExists(t *testing.T) err = s.PostCommit() require.NoError(t, err) - // Verify the ACTIVE session's phase is now ACTIVE_COMMITTED + // Verify the ACTIVE session stays ACTIVE (condenses immediately now) activeState, err = s.loadSessionState(activeSessionID) require.NoError(t, err) - assert.Equal(t, session.PhaseActiveCommitted, activeState.Phase, - "ACTIVE session should transition to ACTIVE_COMMITTED on GitCommit") + assert.Equal(t, session.PhaseActive, activeState.Phase, + "ACTIVE session should stay ACTIVE on GitCommit (condenses immediately)") - // Verify the IDLE session actually condensed (entire/checkpoints/v1 branch should exist) + // Verify the IDLE session also condensed (entire/checkpoints/v1 branch should exist) idleState, err = s.loadSessionState(idleSessionID) require.NoError(t, err) sessionsRef, err := repo.Reference(plumbing.NewBranchReferenceName(paths.MetadataBranchName), true) - require.NoError(t, err, "entire/checkpoints/v1 branch should exist after IDLE session condensation") + require.NoError(t, err, "entire/checkpoints/v1 branch should exist after condensation") require.NotNil(t, sessionsRef) // Verify IDLE session's StepCount was reset by condensation assert.Equal(t, 0, idleState.StepCount, "IDLE session StepCount should be reset after condensation") - - // Verify shadow branch is NOT deleted because the ACTIVE session still needs it. - // After PostCommit, BaseCommit is updated to new HEAD via migration. - newShadowBranch := getShadowBranchNameForCommit(activeState.BaseCommit, activeState.WorktreeID) - refName := plumbing.NewBranchReferenceName(newShadowBranch) - _, err = repo.Reference(refName, true) - assert.NoError(t, err, - "shadow branch should be preserved when an active session still exists on it") } // TestPostCommit_CondensationFailure_PreservesShadowBranch verifies that when @@ -544,74 +546,6 @@ func TestPostCommit_EndedSession_NoFilesTouched_Discards(t *testing.T) { "ENDED session should stay ENDED on discard path") } -// TestPostCommit_ActiveCommitted_MigratesShadowBranch verifies that an -// ACTIVE_COMMITTED session receiving another commit migrates the shadow branch -// to the new HEAD and stays in ACTIVE_COMMITTED. -func TestPostCommit_ActiveCommitted_MigratesShadowBranch(t *testing.T) { - dir := setupGitRepo(t) - t.Chdir(dir) - - repo, err := git.PlainOpen(dir) - require.NoError(t, err) - - s := &ManualCommitStrategy{} - sessionID := "test-postcommit-ac-migrate" - - // Initialize session and save a checkpoint - setupSessionWithCheckpoint(t, s, repo, dir, sessionID) - - // Set phase to ACTIVE_COMMITTED - state, err := s.loadSessionState(sessionID) - require.NoError(t, err) - state.Phase = session.PhaseActiveCommitted - require.NoError(t, s.saveSessionState(state)) - - // Record original shadow branch name and BaseCommit - originalShadowBranch := getShadowBranchNameForCommit(state.BaseCommit, state.WorktreeID) - originalStepCount := state.StepCount - - // Create a commit with the checkpoint trailer - commitWithCheckpointTrailer(t, repo, dir, "d4e5f6a1b2c4") - - // Run PostCommit - err = s.PostCommit() - require.NoError(t, err) - - // Verify phase stays ACTIVE_COMMITTED - state, err = s.loadSessionState(sessionID) - require.NoError(t, err) - assert.Equal(t, session.PhaseActiveCommitted, state.Phase, - "ACTIVE_COMMITTED session should stay ACTIVE_COMMITTED on subsequent commit") - - // Verify BaseCommit updated to new HEAD - head, err := repo.Head() - require.NoError(t, err) - assert.Equal(t, head.Hash().String(), state.BaseCommit, - "BaseCommit should be updated to new HEAD after migration") - - // Verify new shadow branch exists at new HEAD hash - newShadowBranch := getShadowBranchNameForCommit(state.BaseCommit, state.WorktreeID) - newRefName := plumbing.NewBranchReferenceName(newShadowBranch) - _, err = repo.Reference(newRefName, true) - require.NoError(t, err, - "new shadow branch should exist after migration") - - // Verify original shadow branch no longer exists (was migrated/renamed) - oldRefName := plumbing.NewBranchReferenceName(originalShadowBranch) - _, err = repo.Reference(oldRefName, true) - require.Error(t, err, - "original shadow branch should no longer exist after migration") - - // StepCount unchanged (no condensation) - assert.Equal(t, originalStepCount, state.StepCount, - "StepCount should be unchanged - no condensation for ACTIVE_COMMITTED") - - // entire/checkpoints/v1 branch should NOT exist (no condensation) - _, err = repo.Reference(plumbing.NewBranchReferenceName(paths.MetadataBranchName), true) - require.Error(t, err, - "entire/checkpoints/v1 branch should NOT exist - no condensation for ACTIVE_COMMITTED") -} - // TestPostCommit_CondensationFailure_EndedSession_PreservesShadowBranch verifies // that when condensation fails for an ENDED session with files touched, // BaseCommit is preserved (not updated). @@ -671,92 +605,8 @@ func TestPostCommit_CondensationFailure_EndedSession_PreservesShadowBranch(t *te "ENDED session should stay ENDED when condensation fails") } -// TestPostCommit_ActiveSession_SetsPendingCheckpointID verifies that PostCommit -// stores PendingCheckpointID when transitioning ACTIVE → ACTIVE_COMMITTED. -// This ensures HandleTurnEnd can reuse the same checkpoint ID that's in the -// commit trailer, rather than generating a mismatched one. -func TestPostCommit_ActiveSession_SetsPendingCheckpointID(t *testing.T) { - dir := setupGitRepo(t) - t.Chdir(dir) - - repo, err := git.PlainOpen(dir) - require.NoError(t, err) - - s := &ManualCommitStrategy{} - sessionID := "test-postcommit-pending-cpid" - - setupSessionWithCheckpoint(t, s, repo, dir, sessionID) - - // Set phase to ACTIVE (agent mid-turn) - state, err := s.loadSessionState(sessionID) - require.NoError(t, err) - state.Phase = session.PhaseActive - state.PendingCheckpointID = "" // Ensure it starts empty - require.NoError(t, s.saveSessionState(state)) - - // Create a commit with a known checkpoint ID - commitWithCheckpointTrailer(t, repo, dir, "a1b2c3d4e5f6") - - // Run PostCommit - err = s.PostCommit() - require.NoError(t, err) - - // Verify phase transitioned to ACTIVE_COMMITTED - state, err = s.loadSessionState(sessionID) - require.NoError(t, err) - require.Equal(t, session.PhaseActiveCommitted, state.Phase) - - // Verify PendingCheckpointID was stored from the commit trailer - assert.Equal(t, "a1b2c3d4e5f6", state.PendingCheckpointID, - "PendingCheckpointID should be set to the commit's checkpoint ID for deferred condensation") -} - -// TestTurnEnd_ActiveCommitted_ReusesCheckpointID verifies that HandleTurnEnd -// uses PendingCheckpointID (set by PostCommit) rather than generating a new one. -// This ensures the condensed metadata matches the commit trailer. -func TestTurnEnd_ActiveCommitted_ReusesCheckpointID(t *testing.T) { - dir := setupGitRepo(t) - t.Chdir(dir) - - repo, err := git.PlainOpen(dir) - require.NoError(t, err) - - s := &ManualCommitStrategy{} - sessionID := "test-turnend-reuses-cpid" - - setupSessionWithCheckpoint(t, s, repo, dir, sessionID) - - // Simulate PostCommit: transition to ACTIVE_COMMITTED with PendingCheckpointID - state, err := s.loadSessionState(sessionID) - require.NoError(t, err) - state.Phase = session.PhaseActive - require.NoError(t, s.saveSessionState(state)) - - commitWithCheckpointTrailer(t, repo, dir, "d4e5f6a1b2c3") - - err = s.PostCommit() - require.NoError(t, err) - - state, err = s.loadSessionState(sessionID) - require.NoError(t, err) - require.Equal(t, "d4e5f6a1b2c3", state.PendingCheckpointID) - - // Run TurnEnd - result := session.Transition(state.Phase, session.EventTurnEnd, session.TransitionContext{}) - remaining := session.ApplyCommonActions(state, result) - - err = s.HandleTurnEnd(state, remaining) - require.NoError(t, err) - - // Verify the condensed checkpoint ID matches the commit trailer - // The LastCheckpointID is set by condenseAndUpdateState on success - assert.Equal(t, id.CheckpointID("d4e5f6a1b2c3"), state.LastCheckpointID, - "condensation should use PendingCheckpointID, not generate a new one") -} - -// TestTurnEnd_ConcurrentSession_PreservesShadowBranch verifies that -// HandleTurnEnd does NOT delete the shadow branch when another active -// session shares it. +// TestTurnEnd_ConcurrentSession_PreservesShadowBranch verifies that turn +// end with no strategy-specific actions does not interfere with other sessions. func TestTurnEnd_ConcurrentSession_PreservesShadowBranch(t *testing.T) { dir := setupGitRepo(t) t.Chdir(dir) @@ -775,13 +625,13 @@ func TestTurnEnd_ConcurrentSession_PreservesShadowBranch(t *testing.T) { state1, err := s.loadSessionState(sessionID1) require.NoError(t, err) worktreePath := state1.WorktreePath - baseCommit := state1.BaseCommit worktreeID := state1.WorktreeID - // Transition first session through PostCommit to ACTIVE_COMMITTED + // Set first session to ACTIVE state1.Phase = session.PhaseActive require.NoError(t, s.saveSessionState(state1)) + // Trigger PostCommit which condenses immediately (ACTIVE + GitCommit) commitWithCheckpointTrailer(t, repo, dir, "e5f6a1b2c3d4") err = s.PostCommit() @@ -789,13 +639,14 @@ func TestTurnEnd_ConcurrentSession_PreservesShadowBranch(t *testing.T) { state1, err = s.loadSessionState(sessionID1) require.NoError(t, err) - require.Equal(t, session.PhaseActiveCommitted, state1.Phase) + // ACTIVE + GitCommit stays ACTIVE (condenses immediately) + require.Equal(t, session.PhaseActive, state1.Phase) // Create a second session with the SAME base commit and worktree (concurrent) now := time.Now() state2 := &SessionState{ SessionID: sessionID2, - BaseCommit: state1.BaseCommit, // Same base commit (post-migration) + BaseCommit: state1.BaseCommit, // Same base commit (post-condensation) WorktreePath: worktreePath, WorktreeID: worktreeID, StartedAt: now, @@ -805,26 +656,16 @@ func TestTurnEnd_ConcurrentSession_PreservesShadowBranch(t *testing.T) { } require.NoError(t, s.saveSessionState(state2)) - // Record shadow branch name (shared by both sessions) - shadowBranch := getShadowBranchNameForCommit(state1.BaseCommit, state1.WorktreeID) - - // First session ends its turn — should NOT delete shadow branch + // First session ends its turn (ACTIVE -> IDLE, no strategy-specific actions) result := session.Transition(state1.Phase, session.EventTurnEnd, session.TransitionContext{}) remaining := session.ApplyCommonActions(state1, result) + // ACTIVE -> IDLE has no strategy-specific actions + assert.Empty(t, remaining, "ACTIVE + TurnEnd should emit no strategy-specific actions") + err = s.HandleTurnEnd(state1, remaining) require.NoError(t, err) - // Shadow branch at the pre-condensation BaseCommit should be preserved - // because session2 is still active on it. - refName := plumbing.NewBranchReferenceName(shadowBranch) - _, err = repo.Reference(refName, true) - require.NoError(t, err, - "shadow branch should be preserved when another active session shares it") - - // Condensation still succeeded for session1 - assert.Equal(t, 0, state1.StepCount, - "StepCount should be reset after condensation") assert.Equal(t, session.PhaseIdle, state1.Phase, "first session should be IDLE after turn end") @@ -833,134 +674,6 @@ func TestTurnEnd_ConcurrentSession_PreservesShadowBranch(t *testing.T) { require.NoError(t, err) assert.Equal(t, session.PhaseActive, state2.Phase, "second session should still be ACTIVE") - _ = baseCommit // used for documentation -} - -// TestTurnEnd_ActiveCommitted_CondensesSession verifies that HandleTurnEnd -// with ActionCondense (from ACTIVE_COMMITTED → IDLE) condenses the session -// to entire/checkpoints/v1 and cleans up the shadow branch. -func TestTurnEnd_ActiveCommitted_CondensesSession(t *testing.T) { - dir := setupGitRepo(t) - t.Chdir(dir) - - repo, err := git.PlainOpen(dir) - require.NoError(t, err) - - s := &ManualCommitStrategy{} - sessionID := "test-turnend-condenses" - - // Initialize session and save a checkpoint so there is shadow branch content - setupSessionWithCheckpoint(t, s, repo, dir, sessionID) - - // Simulate PostCommit: create a commit with trailer and transition to ACTIVE_COMMITTED - state, err := s.loadSessionState(sessionID) - require.NoError(t, err) - state.Phase = session.PhaseActive - require.NoError(t, s.saveSessionState(state)) - - commitWithCheckpointTrailer(t, repo, dir, "a1b2c3d4e5f6") - - // Run PostCommit so phase transitions to ACTIVE_COMMITTED and PendingCheckpointID is set - err = s.PostCommit() - require.NoError(t, err) - - state, err = s.loadSessionState(sessionID) - require.NoError(t, err) - require.Equal(t, session.PhaseActiveCommitted, state.Phase) - - // Now simulate the TurnEnd transition that the handler dispatches - result := session.Transition(state.Phase, session.EventTurnEnd, session.TransitionContext{}) - remaining := session.ApplyCommonActions(state, result) - - // Verify the state machine emits ActionCondense - require.Contains(t, remaining, session.ActionCondense, - "ACTIVE_COMMITTED + TurnEnd should emit ActionCondense") - - // Record shadow branch name BEFORE HandleTurnEnd (BaseCommit may change) - shadowBranch := getShadowBranchNameForCommit(state.BaseCommit, state.WorktreeID) - - // Call HandleTurnEnd with the remaining actions - err = s.HandleTurnEnd(state, remaining) - require.NoError(t, err) - - // Verify condensation happened: entire/checkpoints/v1 branch should exist - sessionsRef, err := repo.Reference(plumbing.NewBranchReferenceName(paths.MetadataBranchName), true) - require.NoError(t, err, "entire/checkpoints/v1 branch should exist after turn-end condensation") - assert.NotNil(t, sessionsRef) - - // Verify shadow branch IS deleted after condensation - refName := plumbing.NewBranchReferenceName(shadowBranch) - _, err = repo.Reference(refName, true) - require.Error(t, err, - "shadow branch should be deleted after turn-end condensation") - - // Verify StepCount was reset by condensation - assert.Equal(t, 0, state.StepCount, - "StepCount should be reset after condensation") - - // Verify phase is IDLE (set by ApplyCommonActions above) - assert.Equal(t, session.PhaseIdle, state.Phase, - "phase should be IDLE after TurnEnd") -} - -// TestTurnEnd_ActiveCommitted_CondensationFailure_PreservesShadowBranch verifies -// that when HandleTurnEnd condensation fails, BaseCommit is NOT updated and -// the shadow branch is preserved. -func TestTurnEnd_ActiveCommitted_CondensationFailure_PreservesShadowBranch(t *testing.T) { - dir := setupGitRepo(t) - t.Chdir(dir) - - repo, err := git.PlainOpen(dir) - require.NoError(t, err) - - s := &ManualCommitStrategy{} - sessionID := "test-turnend-condense-fail" - - // Initialize session and save a checkpoint - setupSessionWithCheckpoint(t, s, repo, dir, sessionID) - - // Simulate PostCommit: transition to ACTIVE_COMMITTED - state, err := s.loadSessionState(sessionID) - require.NoError(t, err) - state.Phase = session.PhaseActive - require.NoError(t, s.saveSessionState(state)) - - commitWithCheckpointTrailer(t, repo, dir, "b2c3d4e5f6a1") - - err = s.PostCommit() - require.NoError(t, err) - - state, err = s.loadSessionState(sessionID) - require.NoError(t, err) - require.Equal(t, session.PhaseActiveCommitted, state.Phase) - - // Record original BaseCommit before corruption - originalBaseCommit := state.BaseCommit - originalStepCount := state.StepCount - - // Corrupt shadow branch by pointing it at ZeroHash - shadowBranch := getShadowBranchNameForCommit(state.BaseCommit, state.WorktreeID) - corruptRef := plumbing.NewHashReference(plumbing.NewBranchReferenceName(shadowBranch), plumbing.ZeroHash) - require.NoError(t, repo.Storer.SetReference(corruptRef)) - - // Run the transition - result := session.Transition(state.Phase, session.EventTurnEnd, session.TransitionContext{}) - remaining := session.ApplyCommonActions(state, result) - - // Call HandleTurnEnd — condensation should fail silently - err = s.HandleTurnEnd(state, remaining) - require.NoError(t, err, "HandleTurnEnd should not return error even when condensation fails") - - // BaseCommit should NOT be updated (condensation failed) - assert.Equal(t, originalBaseCommit, state.BaseCommit, - "BaseCommit should NOT be updated when condensation fails") - assert.Equal(t, originalStepCount, state.StepCount, - "StepCount should NOT be reset when condensation fails") - - // entire/checkpoints/v1 branch should NOT exist (condensation failed) - _, err = repo.Reference(plumbing.NewBranchReferenceName(paths.MetadataBranchName), true) - assert.Error(t, err, - "entire/checkpoints/v1 branch should NOT exist when condensation fails") } // TestTurnEnd_Active_NoActions verifies that HandleTurnEnd with no actions @@ -1013,97 +726,6 @@ func TestTurnEnd_Active_NoActions(t *testing.T) { "shadow branch should still exist after no-op turn end") } -// TestTurnEnd_DeferredCondensation_AttributionUsesOriginalBase verifies that -// deferred condensation (ACTIVE_COMMITTED → IDLE) uses AttributionBaseCommit -// instead of BaseCommit for attribution, so the diff is non-zero. -// -// Scenario: agent modifies a file, user commits mid-turn, then turn ends. -// Without the fix, BaseCommit is updated to the new HEAD by PostCommit migration, -// so baseTree == headTree and attribution shows zero changes. -// With the fix, AttributionBaseCommit preserves the original base commit. -func TestTurnEnd_DeferredCondensation_AttributionUsesOriginalBase(t *testing.T) { - dir := setupGitRepo(t) - t.Chdir(dir) - - repo, err := git.PlainOpen(dir) - require.NoError(t, err) - - s := &ManualCommitStrategy{} - sessionID := "test-deferred-attribution" - - // Initialize session and save a checkpoint that includes a modified file. - // The "agent" modifies test.txt before saving the checkpoint. - setupSessionWithFileChange(t, s, repo, dir, sessionID) - - // Record the original base commit (commit A) - state, err := s.loadSessionState(sessionID) - require.NoError(t, err) - originalBaseCommit := state.BaseCommit - - // Verify AttributionBaseCommit is set at session init - assert.Equal(t, originalBaseCommit, state.AttributionBaseCommit, - "AttributionBaseCommit should equal BaseCommit at session start") - - // Set phase to ACTIVE (simulating agent mid-turn) - state.Phase = session.PhaseActive - state.FilesTouched = []string{"test.txt"} - require.NoError(t, s.saveSessionState(state)) - - // User commits (creates commit B). This triggers PostCommit which: - // - Transitions ACTIVE → ACTIVE_COMMITTED (defers condensation) - // - Migrates shadow branch to new HEAD - // - Updates BaseCommit to new HEAD - commitWithCheckpointTrailer(t, repo, dir, "a1b2c3d4e5f6") - - err = s.PostCommit() - require.NoError(t, err) - - // Reload state and verify the key invariant: - // BaseCommit has moved to the new HEAD, but AttributionBaseCommit stays at original - state, err = s.loadSessionState(sessionID) - require.NoError(t, err) - require.Equal(t, session.PhaseActiveCommitted, state.Phase) - - head, err := repo.Head() - require.NoError(t, err) - newHeadHash := head.Hash().String() - - assert.Equal(t, newHeadHash, state.BaseCommit, - "BaseCommit should be updated to new HEAD after migration") - assert.Equal(t, originalBaseCommit, state.AttributionBaseCommit, - "AttributionBaseCommit should still point to original base (commit A)") - assert.NotEqual(t, state.BaseCommit, state.AttributionBaseCommit, - "BaseCommit and AttributionBaseCommit should diverge after mid-turn commit") - - // Now simulate TurnEnd (agent finishes) — deferred condensation runs - result := session.Transition(state.Phase, session.EventTurnEnd, session.TransitionContext{}) - remaining := session.ApplyCommonActions(state, result) - require.Contains(t, remaining, session.ActionCondense) - - err = s.HandleTurnEnd(state, remaining) - require.NoError(t, err) - - // After condensation, verify AttributionBaseCommit is updated to match BaseCommit - assert.Equal(t, state.BaseCommit, state.AttributionBaseCommit, - "AttributionBaseCommit should be updated after successful condensation") - - // Verify condensation actually happened - _, err = repo.Reference(plumbing.NewBranchReferenceName(paths.MetadataBranchName), true) - require.NoError(t, err, "entire/sessions branch should exist after deferred condensation") - - // Read back the committed metadata and verify attribution is non-zero. - // The agent modified test.txt (added a line), so AgentLines should be > 0. - store := checkpoint.NewGitStore(repo) - cpID := id.MustCheckpointID("a1b2c3d4e5f6") - content, err := store.ReadSessionContent(context.Background(), cpID, 0) - require.NoError(t, err, "should be able to read condensed session content") - require.NotNil(t, content) - require.NotNil(t, content.Metadata.InitialAttribution, - "condensed metadata should include attribution") - assert.Positive(t, content.Metadata.InitialAttribution.TotalCommitted, - "attribution TotalCommitted should be non-zero (agent modified test.txt)") -} - // TestPostCommit_FilesTouched_ResetsAfterCondensation verifies that FilesTouched // is reset after condensation, so subsequent condensations only contain the files // touched since the last commit — not the accumulated history. @@ -1240,44 +862,6 @@ func TestPostCommit_FilesTouched_ResetsAfterCondensation(t *testing.T) { "Second condensation should only contain C.txt and D.txt, not accumulated files from first condensation") } -// setupSessionWithFileChange is like setupSessionWithCheckpoint but also modifies -// test.txt so the shadow branch checkpoint includes actual file changes. -// This enables attribution testing: the diff between base commit and the -// checkpoint/HEAD shows real line changes. -func setupSessionWithFileChange(t *testing.T, s *ManualCommitStrategy, _ *git.Repository, dir, sessionID string) { - t.Helper() - - // Modify test.txt to simulate agent work (adds lines relative to initial commit) - testFile := filepath.Join(dir, "test.txt") - require.NoError(t, os.WriteFile(testFile, []byte("initial content\nagent added line\n"), 0o644)) - - // Create metadata directory with a transcript file - metadataDir := ".entire/metadata/" + sessionID - metadataDirAbs := filepath.Join(dir, metadataDir) - require.NoError(t, os.MkdirAll(metadataDirAbs, 0o755)) - - transcript := `{"type":"human","message":{"content":"test prompt"}} -{"type":"assistant","message":{"content":"test response"}} -` - require.NoError(t, os.WriteFile( - filepath.Join(metadataDirAbs, paths.TranscriptFileName), - []byte(transcript), 0o644)) - - // SaveChanges creates the shadow branch and checkpoint - err := s.SaveChanges(SaveContext{ - SessionID: sessionID, - ModifiedFiles: []string{"test.txt"}, - NewFiles: []string{}, - DeletedFiles: []string{}, - MetadataDir: metadataDir, - MetadataDirAbs: metadataDirAbs, - CommitMessage: "Checkpoint 1", - AuthorName: "Test", - AuthorEmail: "test@test.com", - }) - require.NoError(t, err, "SaveChanges should succeed to create shadow branch content") -} - // setupSessionWithCheckpoint initializes a session and creates one checkpoint // on the shadow branch so there is content available for condensation. func setupSessionWithCheckpoint(t *testing.T, s *ManualCommitStrategy, _ *git.Repository, dir, sessionID string) { diff --git a/cmd/entire/cli/strategy/phase_prepare_commit_msg_test.go b/cmd/entire/cli/strategy/phase_prepare_commit_msg_test.go index 377708122..38541d1fb 100644 --- a/cmd/entire/cli/strategy/phase_prepare_commit_msg_test.go +++ b/cmd/entire/cli/strategy/phase_prepare_commit_msg_test.go @@ -4,17 +4,10 @@ import ( "os" "path/filepath" "testing" - "time" "github.com/entireio/cli/cmd/entire/cli/agent" - "github.com/entireio/cli/cmd/entire/cli/checkpoint" - "github.com/entireio/cli/cmd/entire/cli/paths" - "github.com/entireio/cli/cmd/entire/cli/session" "github.com/entireio/cli/cmd/entire/cli/trailers" - "github.com/go-git/go-git/v5" - "github.com/go-git/go-git/v5/plumbing" - "github.com/go-git/go-git/v5/plumbing/object" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -51,11 +44,11 @@ func TestPrepareCommitMsg_AmendPreservesExistingTrailer(t *testing.T) { "trailer should preserve the original checkpoint ID") } -// TestPrepareCommitMsg_AmendRestoresTrailerFromPendingCheckpointID verifies the amend -// bug fix: when a user does `git commit --amend -m "new message"`, the Entire-Checkpoint +// TestPrepareCommitMsg_AmendRestoresTrailerFromLastCheckpointID verifies the amend +// restore: when a user does `git commit --amend -m "new message"`, the Entire-Checkpoint // trailer is lost because the new message replaces the old one. PrepareCommitMsg restores -// the trailer from PendingCheckpointID in session state. -func TestPrepareCommitMsg_AmendRestoresTrailerFromPendingCheckpointID(t *testing.T) { +// the trailer from LastCheckpointID in session state. +func TestPrepareCommitMsg_AmendRestoresTrailerFromLastCheckpointID(t *testing.T) { dir := setupGitRepo(t) t.Chdir(dir) @@ -65,11 +58,11 @@ func TestPrepareCommitMsg_AmendRestoresTrailerFromPendingCheckpointID(t *testing err := s.InitializeSession(sessionID, agent.AgentTypeClaudeCode, "", "") require.NoError(t, err) - // Simulate state after condensation: PendingCheckpointID is set + // Simulate state after condensation: LastCheckpointID is set state, err := s.loadSessionState(sessionID) require.NoError(t, err) require.NotNil(t, state) - state.PendingCheckpointID = "abc123def456" + state.LastCheckpointID = "abc123def456" err = s.saveSessionState(state) require.NoError(t, err) @@ -82,21 +75,21 @@ func TestPrepareCommitMsg_AmendRestoresTrailerFromPendingCheckpointID(t *testing err = s.PrepareCommitMsg(commitMsgFile, "commit") require.NoError(t, err) - // Read the file back - trailer should be restored from PendingCheckpointID + // Read the file back - trailer should be restored from LastCheckpointID content, err := os.ReadFile(commitMsgFile) require.NoError(t, err) cpID, found := trailers.ParseCheckpoint(string(content)) assert.True(t, found, - "trailer should be restored from PendingCheckpointID on amend") + "trailer should be restored from LastCheckpointID on amend") assert.Equal(t, "abc123def456", cpID.String(), - "restored trailer should use PendingCheckpointID value") + "restored trailer should use LastCheckpointID value") } -// TestPrepareCommitMsg_AmendNoTrailerNoPendingID verifies that when amending with -// no existing trailer and no PendingCheckpointID in session state, no trailer is added. +// TestPrepareCommitMsg_AmendNoTrailerNoLastID verifies that when amending with +// no existing trailer and no LastCheckpointID in session state, no trailer is added. // This is the case where the session has never been condensed yet. -func TestPrepareCommitMsg_AmendNoTrailerNoPendingID(t *testing.T) { +func TestPrepareCommitMsg_AmendNoTrailerNoLastID(t *testing.T) { dir := setupGitRepo(t) t.Chdir(dir) @@ -106,11 +99,11 @@ func TestPrepareCommitMsg_AmendNoTrailerNoPendingID(t *testing.T) { err := s.InitializeSession(sessionID, agent.AgentTypeClaudeCode, "", "") require.NoError(t, err) - // Verify PendingCheckpointID is empty (default) + // Verify LastCheckpointID is empty (default) state, err := s.loadSessionState(sessionID) require.NoError(t, err) require.NotNil(t, state) - assert.Empty(t, state.PendingCheckpointID, "PendingCheckpointID should be empty by default") + assert.True(t, state.LastCheckpointID.IsEmpty(), "LastCheckpointID should be empty by default") // Write a commit message file with NO trailer commitMsgFile := filepath.Join(t.TempDir(), "COMMIT_EDITMSG") @@ -127,177 +120,9 @@ func TestPrepareCommitMsg_AmendNoTrailerNoPendingID(t *testing.T) { _, found := trailers.ParseCheckpoint(string(content)) assert.False(t, found, - "no trailer should be added when PendingCheckpointID is empty") + "no trailer should be added when LastCheckpointID is empty") // Message should be unchanged assert.Equal(t, newMsg, string(content), "commit message should be unchanged when no trailer to restore") } - -// TestPrepareCommitMsg_NormalCommitUsesPendingCheckpointID verifies that during -// a normal commit (source=""), if the session is in ACTIVE_COMMITTED phase with -// a PendingCheckpointID, the pending ID is reused instead of generating a new one. -// This ensures idempotent checkpoint IDs across prepare-commit-msg invocations. -func TestPrepareCommitMsg_NormalCommitUsesPendingCheckpointID(t *testing.T) { - dir := setupGitRepo(t) - t.Chdir(dir) - - s := &ManualCommitStrategy{} - - sessionID := "test-session-normal-pending" - err := s.InitializeSession(sessionID, agent.AgentTypeClaudeCode, "", "") - require.NoError(t, err) - - // Create content on the shadow branch so filterSessionsWithNewContent finds it - createShadowBranchWithTranscript(t, dir, sessionID) - - // Set the session to ACTIVE_COMMITTED with a PendingCheckpointID - state, err := s.loadSessionState(sessionID) - require.NoError(t, err) - require.NotNil(t, state) - state.Phase = session.PhaseActiveCommitted - state.PendingCheckpointID = "fedcba987654" - // Ensure StepCount reflects that a checkpoint exists on the shadow branch - state.StepCount = 1 - err = s.saveSessionState(state) - require.NoError(t, err) - - // Write a commit message file with no trailer (normal editor flow) - commitMsgFile := filepath.Join(t.TempDir(), "COMMIT_EDITMSG") - normalMsg := "Feature: add new functionality\n" - require.NoError(t, os.WriteFile(commitMsgFile, []byte(normalMsg), 0o644)) - - // Call PrepareCommitMsg with source="" (normal commit, editor flow) - err = s.PrepareCommitMsg(commitMsgFile, "") - require.NoError(t, err) - - // Read the file back - trailer should use PendingCheckpointID - content, err := os.ReadFile(commitMsgFile) - require.NoError(t, err) - - cpID, found := trailers.ParseCheckpoint(string(content)) - assert.True(t, found, - "trailer should be present for normal commit with active session content") - assert.Equal(t, "fedcba987654", cpID.String(), - "normal commit should reuse PendingCheckpointID instead of generating a new one") -} - -// createShadowBranchWithTranscript creates a shadow branch commit with a minimal -// transcript file so that filterSessionsWithNewContent detects new content. -// This uses low-level go-git plumbing to create the branch directly. -func createShadowBranchWithTranscript(t *testing.T, repoDir string, sessionID string) { - t.Helper() - - repo, err := git.PlainOpen(repoDir) - require.NoError(t, err) - - head, err := repo.Head() - require.NoError(t, err) - baseCommit := head.Hash().String() - - // Build the tree with a transcript file at the expected path - metadataDir := paths.EntireMetadataDir + "/" + sessionID - transcriptPath := metadataDir + "/" + paths.TranscriptFileName - transcriptContent := `{"type":"message","role":"assistant","content":"hello"}` + "\n" - - // Create blob for transcript - blobObj := &plumbing.MemoryObject{} - blobObj.SetType(plumbing.BlobObject) - blobObj.SetSize(int64(len(transcriptContent))) - writer, err := blobObj.Writer() - require.NoError(t, err) - _, err = writer.Write([]byte(transcriptContent)) - require.NoError(t, err) - require.NoError(t, writer.Close()) - - blobHash, err := repo.Storer.SetEncodedObject(blobObj) - require.NoError(t, err) - - // Build nested tree structure: .entire/metadata//full.jsonl - // We need to build trees bottom-up - innerTree := object.Tree{ - Entries: []object.TreeEntry{ - {Name: paths.TranscriptFileName, Mode: 0o100644, Hash: blobHash}, - }, - } - innerTreeObj := repo.Storer.NewEncodedObject() - innerTreeObj.SetType(plumbing.TreeObject) - require.NoError(t, innerTree.Encode(innerTreeObj)) - innerTreeHash, err := repo.Storer.SetEncodedObject(innerTreeObj) - require.NoError(t, err) - - // Build .entire/metadata/ level - sessionTree := object.Tree{ - Entries: []object.TreeEntry{ - {Name: sessionID, Mode: 0o040000, Hash: innerTreeHash}, - }, - } - sessionTreeObj := repo.Storer.NewEncodedObject() - sessionTreeObj.SetType(plumbing.TreeObject) - require.NoError(t, sessionTree.Encode(sessionTreeObj)) - sessionTreeHash, err := repo.Storer.SetEncodedObject(sessionTreeObj) - require.NoError(t, err) - - // Build .entire/metadata level - metadataTree := object.Tree{ - Entries: []object.TreeEntry{ - {Name: "metadata", Mode: 0o040000, Hash: sessionTreeHash}, - }, - } - metadataTreeObj := repo.Storer.NewEncodedObject() - metadataTreeObj.SetType(plumbing.TreeObject) - require.NoError(t, metadataTree.Encode(metadataTreeObj)) - metadataTreeHash, err := repo.Storer.SetEncodedObject(metadataTreeObj) - require.NoError(t, err) - - // Build .entire level - entireTree := object.Tree{ - Entries: []object.TreeEntry{ - {Name: ".entire", Mode: 0o040000, Hash: metadataTreeHash}, - }, - } - entireTreeObj := repo.Storer.NewEncodedObject() - entireTreeObj.SetType(plumbing.TreeObject) - require.NoError(t, entireTree.Encode(entireTreeObj)) - entireTreeHash, err := repo.Storer.SetEncodedObject(entireTreeObj) - require.NoError(t, err) - - // Create commit on shadow branch - now := time.Now() - commitObj := &object.Commit{ - Author: object.Signature{ - Name: "Test", - Email: "test@test.com", - When: now, - }, - Committer: object.Signature{ - Name: "Test", - Email: "test@test.com", - When: now, - }, - Message: "checkpoint\n\nEntire-Metadata: " + metadataDir + "\nEntire-Session: " + sessionID + "\nEntire-Strategy: manual-commit\n", - TreeHash: entireTreeHash, - } - commitEnc := repo.Storer.NewEncodedObject() - require.NoError(t, commitObj.Encode(commitEnc)) - commitHash, err := repo.Storer.SetEncodedObject(commitEnc) - require.NoError(t, err) - - // Create the shadow branch reference - // WorktreeID is empty for main worktree, which matches what setupGitRepo creates - shadowBranchName := checkpoint.ShadowBranchNameForCommit(baseCommit, "") - refName := plumbing.NewBranchReferenceName(shadowBranchName) - ref := plumbing.NewHashReference(refName, commitHash) - require.NoError(t, repo.Storer.SetReference(ref)) - - // Verify the transcript is readable - verifyCommit, err := repo.CommitObject(commitHash) - require.NoError(t, err) - verifyTree, err := verifyCommit.Tree() - require.NoError(t, err) - file, err := verifyTree.File(transcriptPath) - require.NoError(t, err, "transcript file should exist at %s", transcriptPath) - content, err := file.Contents() - require.NoError(t, err) - require.NotEmpty(t, content, "transcript should have content") -} diff --git a/cmd/entire/cli/strategy/phase_wiring_test.go b/cmd/entire/cli/strategy/phase_wiring_test.go index 6795ec03c..a91764212 100644 --- a/cmd/entire/cli/strategy/phase_wiring_test.go +++ b/cmd/entire/cli/strategy/phase_wiring_test.go @@ -136,9 +136,10 @@ func TestInitializeSession_EndedToActive(t *testing.T) { require.NotNil(t, state.LastInteractionTime) } -// TestInitializeSession_ActiveCommittedToActive verifies Ctrl-C recovery -// after a mid-session commit: ACTIVE_COMMITTED → ACTIVE. -func TestInitializeSession_ActiveCommittedToActive(t *testing.T) { +// TestInitializeSession_OldActiveCommittedToActive verifies backward compatibility: +// if a state file from an older CLI version has phase="active_committed", +// PhaseFromString normalizes it to ACTIVE, so the next TurnStart stays ACTIVE. +func TestInitializeSession_OldActiveCommittedToActive(t *testing.T) { dir := setupGitRepo(t) t.Chdir(dir) @@ -148,21 +149,22 @@ func TestInitializeSession_ActiveCommittedToActive(t *testing.T) { err := s.InitializeSession("test-session-ac-recovery", "Claude Code", "", "") require.NoError(t, err) - // Manually set to ACTIVE_COMMITTED + // Manually set phase to the old "active_committed" string (simulating old state file) state, err := s.loadSessionState("test-session-ac-recovery") require.NoError(t, err) - state.Phase = session.PhaseActiveCommitted + state.Phase = "active_committed" // raw string, not using the deprecated var err = s.saveSessionState(state) require.NoError(t, err) - // Call InitializeSession again - should transition ACTIVE_COMMITTED → ACTIVE + // Call InitializeSession again - PhaseFromString normalizes to ACTIVE, + // then TurnStart keeps it ACTIVE err = s.InitializeSession("test-session-ac-recovery", "Claude Code", "", "") require.NoError(t, err) state, err = s.loadSessionState("test-session-ac-recovery") require.NoError(t, err) assert.Equal(t, session.PhaseActive, state.Phase, - "should transition from ACTIVE_COMMITTED to ACTIVE") + "old active_committed phase should normalize to ACTIVE") } // TestInitializeSession_EmptyPhaseBackwardCompat verifies that sessions diff --git a/cmd/entire/cli/strategy/strategy.go b/cmd/entire/cli/strategy/strategy.go index 37fdc79c4..41184b8d2 100644 --- a/cmd/entire/cli/strategy/strategy.go +++ b/cmd/entire/cli/strategy/strategy.go @@ -458,12 +458,10 @@ type PrePushHandler interface { } // TurnEndHandler is an optional interface for strategies that need to -// handle deferred actions when an agent turn ends. -// For example, manual-commit strategy uses this to condense session data -// that was deferred during ACTIVE_COMMITTED → IDLE transitions. +// handle actions when an agent turn ends (ACTIVE → IDLE). type TurnEndHandler interface { // HandleTurnEnd dispatches strategy-specific actions emitted by the - // ACTIVE_COMMITTED → IDLE (or other) turn-end transition. + // ACTIVE → IDLE turn-end transition. // The state has already been updated by ApplyCommonActions; the caller // saves it after this method returns. HandleTurnEnd(state *session.State, actions []session.Action) error diff --git a/docs/KNOWN_LIMITATIONS.md b/docs/KNOWN_LIMITATIONS.md index 99ad44d53..a9db3ed63 100644 --- a/docs/KNOWN_LIMITATIONS.md +++ b/docs/KNOWN_LIMITATIONS.md @@ -8,7 +8,7 @@ This document describes known limitations of the Entire CLI. When you amend a commit using `git commit --amend -m "new message"`, the `-m` flag replaces the entire message including any `Entire-Checkpoint` trailer. Git passes `source="message"` (not `"commit"`) to the prepare-commit-msg hook, so the amend-specific trailer preservation logic is bypassed. -**However, the trailer is automatically restored** if `PendingCheckpointID` or `LastCheckpointID` exists in session state (set during the original condensation). This means `git commit --amend -m "..."` preserves the checkpoint link in most cases, including when Claude does the amend in a non-interactive environment. +**However, the trailer is automatically restored** if `LastCheckpointID` exists in session state (set during condensation). This means `git commit --amend -m "..."` preserves the checkpoint link in most cases, including when Claude does the amend in a non-interactive environment. The only case where the link is lost is when `-m` is used with genuinely *new* content (no prior condensation) and `/dev/tty` is not available for the interactive confirmation prompt. diff --git a/docs/architecture/claude-hooks-integration.md b/docs/architecture/claude-hooks-integration.md index 69e7d5602..980a659a6 100644 --- a/docs/architecture/claude-hooks-integration.md +++ b/docs/architecture/claude-hooks-integration.md @@ -120,9 +120,15 @@ Fires when Claude finishes responding. Does **not** fire on user interrupt (Ctrl - **Auto-commit**: Creates a commit on the active branch with the `Entire-Checkpoint` trailer. - Token usage is stored in `metadata.json` for later analysis and reporting. -7. **Update Session State**: Updates `CheckpointTranscriptStart` to track transcript position for detecting new content in future checkpoints (auto-commit strategy only). +7. **Handle Trailing Transcript** (manual-commit strategy): -8. **Cleanup**: Deletes the temporary `.entire/tmp/pre-prompt-.json` file. + - If a git commit occurred during this turn (PostCommit already condensed), the agent may have continued generating conversation afterward (e.g., summarizing what it did). + - If no new files were touched in that trailing conversation, the transcript is appended to the prior committed checkpoint rather than creating a new shadow branch checkpoint. + - This keeps the committed checkpoint's transcript complete without creating unnecessary empty checkpoints. + +8. **Update Session State**: Updates `CheckpointTranscriptStart` to track transcript position for detecting new content in future checkpoints (auto-commit strategy only). + +9. **Cleanup**: Deletes the temporary `.entire/tmp/pre-prompt-.json` file. ### `PreToolUse[Task]`