diff --git a/cmd/entire/cli/agent/claudecode/transcript.go b/cmd/entire/cli/agent/claudecode/transcript.go index de344d7be..06da63377 100644 --- a/cmd/entire/cli/agent/claudecode/transcript.go +++ b/cmd/entire/cli/agent/claudecode/transcript.go @@ -427,3 +427,49 @@ func CalculateTotalTokenUsage(transcriptPath string, startLine int, subagentsDir return mainUsage, nil } + +// ExtractAllModifiedFiles extracts files modified by both the main agent and +// any subagents spawned via the Task tool. It parses the main transcript from +// startLine, collects modified files from the main agent, then reads each +// subagent's transcript from subagentsDir to collect their modified files too. +// The result is a deduplicated list of all modified file paths. +func ExtractAllModifiedFiles(transcriptPath string, startLine int, subagentsDir string) ([]string, error) { + if transcriptPath == "" { + return nil, nil + } + + // Parse main transcript once + transcript, err := parseTranscriptFromLine(transcriptPath, startLine) + if err != nil { + return nil, fmt.Errorf("failed to parse transcript: %w", err) + } + + // Collect modified files from main agent + fileSet := make(map[string]bool) + var files []string + for _, f := range ExtractModifiedFiles(transcript) { + if !fileSet[f] { + fileSet[f] = true + files = append(files, f) + } + } + + // Find spawned subagents and collect their modified files + agentIDs := ExtractSpawnedAgentIDs(transcript) + for agentID := range agentIDs { + agentPath := filepath.Join(subagentsDir, fmt.Sprintf("agent-%s.jsonl", agentID)) + agentTranscript, err := parseTranscriptFromLine(agentPath, 0) + if err != nil { + // Subagent transcript may not exist yet or may have been cleaned up + continue + } + for _, f := range ExtractModifiedFiles(agentTranscript) { + if !fileSet[f] { + fileSet[f] = true + files = append(files, f) + } + } + } + + return files, nil +} diff --git a/cmd/entire/cli/agent/claudecode/transcript_test.go b/cmd/entire/cli/agent/claudecode/transcript_test.go index 7872a04a0..571f658c9 100644 --- a/cmd/entire/cli/agent/claudecode/transcript_test.go +++ b/cmd/entire/cli/agent/claudecode/transcript_test.go @@ -3,6 +3,7 @@ package claudecode import ( "encoding/json" "os" + "strings" "testing" "github.com/entireio/cli/cmd/entire/cli/transcript" @@ -618,3 +619,257 @@ func TestCalculateTotalTokenUsage_PerCheckpoint(t *testing.T) { t.Errorf("From line 4: got APICallCount=%d, want 1", usage3.APICallCount) } } + +// writeJSONLFile is a test helper that writes JSONL transcript lines to a file. +func writeJSONLFile(t *testing.T, path string, lines ...string) { + t.Helper() + var buf strings.Builder + for _, line := range lines { + buf.WriteString(line) + buf.WriteByte('\n') + } + if err := os.WriteFile(path, []byte(buf.String()), 0o600); err != nil { + t.Fatalf("failed to write JSONL file %s: %v", path, err) + } +} + +// makeWriteToolLine returns a JSONL assistant line with a Write tool_use for the given file. +func makeWriteToolLine(t *testing.T, uuid, filePath string) string { + t.Helper() + data := mustMarshal(t, map[string]interface{}{ + "content": []map[string]interface{}{ + { + "type": "tool_use", + "id": "toolu_" + uuid, + "name": "Write", + "input": map[string]string{"file_path": filePath}, + }, + }, + }) + line := mustMarshal(t, map[string]interface{}{ + "type": "assistant", + "uuid": uuid, + "message": json.RawMessage(data), + }) + return string(line) +} + +// makeEditToolLine returns a JSONL assistant line with an Edit tool_use for the given file. +func makeEditToolLine(t *testing.T, uuid, filePath string) string { + t.Helper() + data := mustMarshal(t, map[string]interface{}{ + "content": []map[string]interface{}{ + { + "type": "tool_use", + "id": "toolu_" + uuid, + "name": "Edit", + "input": map[string]string{"file_path": filePath}, + }, + }, + }) + line := mustMarshal(t, map[string]interface{}{ + "type": "assistant", + "uuid": uuid, + "message": json.RawMessage(data), + }) + return string(line) +} + +// makeTaskToolUseLine returns a JSONL assistant line with a Task tool_use (spawning a subagent). +func makeTaskToolUseLine(t *testing.T, uuid, toolUseID string) string { + t.Helper() + data := mustMarshal(t, map[string]interface{}{ + "content": []map[string]interface{}{ + { + "type": "tool_use", + "id": toolUseID, + "name": "Task", + "input": map[string]string{"prompt": "do something"}, + }, + }, + }) + line := mustMarshal(t, map[string]interface{}{ + "type": "assistant", + "uuid": uuid, + "message": json.RawMessage(data), + }) + return string(line) +} + +// makeTaskResultLine returns a JSONL user line with a tool_result containing agentId. +func makeTaskResultLine(t *testing.T, uuid, toolUseID, agentID string) string { + t.Helper() + data := mustMarshal(t, map[string]interface{}{ + "content": []map[string]interface{}{ + { + "type": "tool_result", + "tool_use_id": toolUseID, + "content": "agentId: " + agentID, + }, + }, + }) + line := mustMarshal(t, map[string]interface{}{ + "type": "user", + "uuid": uuid, + "message": json.RawMessage(data), + }) + return string(line) +} + +func TestExtractAllModifiedFiles_IncludesSubagentFiles(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + transcriptPath := tmpDir + "/transcript.jsonl" + subagentsDir := tmpDir + "/tasks/toolu_task1" + + if err := os.MkdirAll(subagentsDir, 0o755); err != nil { + t.Fatalf("failed to create subagents dir: %v", err) + } + + // Main transcript: Write to main.go + Task call spawning subagent "sub1" + writeJSONLFile(t, transcriptPath, + makeWriteToolLine(t, "a1", "/repo/main.go"), + makeTaskToolUseLine(t, "a2", "toolu_task1"), + makeTaskResultLine(t, "u1", "toolu_task1", "sub1"), + ) + + // Subagent transcript: Write to helper.go + Edit to utils.go + writeJSONLFile(t, subagentsDir+"/agent-sub1.jsonl", + makeWriteToolLine(t, "sa1", "/repo/helper.go"), + makeEditToolLine(t, "sa2", "/repo/utils.go"), + ) + + files, err := ExtractAllModifiedFiles(transcriptPath, 0, subagentsDir) + if err != nil { + t.Fatalf("ExtractAllModifiedFiles() error: %v", err) + } + + if len(files) != 3 { + t.Errorf("expected 3 files, got %d: %v", len(files), files) + } + + wantFiles := map[string]bool{ + "/repo/main.go": true, + "/repo/helper.go": true, + "/repo/utils.go": true, + } + for _, f := range files { + if !wantFiles[f] { + t.Errorf("unexpected file %q in result", f) + } + delete(wantFiles, f) + } + for f := range wantFiles { + t.Errorf("missing expected file %q", f) + } +} + +func TestExtractAllModifiedFiles_DeduplicatesAcrossAgents(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + transcriptPath := tmpDir + "/transcript.jsonl" + subagentsDir := tmpDir + "/tasks/toolu_task1" + + if err := os.MkdirAll(subagentsDir, 0o755); err != nil { + t.Fatalf("failed to create subagents dir: %v", err) + } + + // Main transcript: Write to shared.go + Task call + writeJSONLFile(t, transcriptPath, + makeWriteToolLine(t, "a1", "/repo/shared.go"), + makeTaskToolUseLine(t, "a2", "toolu_task1"), + makeTaskResultLine(t, "u1", "toolu_task1", "sub1"), + ) + + // Subagent transcript: Also modifies shared.go (same file as main) + writeJSONLFile(t, subagentsDir+"/agent-sub1.jsonl", + makeEditToolLine(t, "sa1", "/repo/shared.go"), + ) + + files, err := ExtractAllModifiedFiles(transcriptPath, 0, subagentsDir) + if err != nil { + t.Fatalf("ExtractAllModifiedFiles() error: %v", err) + } + + if len(files) != 1 { + t.Errorf("expected 1 file (deduplicated), got %d: %v", len(files), files) + } + if len(files) > 0 && files[0] != "/repo/shared.go" { + t.Errorf("expected /repo/shared.go, got %q", files[0]) + } +} + +func TestExtractAllModifiedFiles_NoSubagents(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + transcriptPath := tmpDir + "/transcript.jsonl" + + // Main transcript: Write to a file, no Task calls + writeJSONLFile(t, transcriptPath, + makeWriteToolLine(t, "a1", "/repo/solo.go"), + ) + + files, err := ExtractAllModifiedFiles(transcriptPath, 0, tmpDir+"/nonexistent") + if err != nil { + t.Fatalf("ExtractAllModifiedFiles() error: %v", err) + } + + if len(files) != 1 { + t.Errorf("expected 1 file, got %d: %v", len(files), files) + } + if len(files) > 0 && files[0] != "/repo/solo.go" { + t.Errorf("expected /repo/solo.go, got %q", files[0]) + } +} + +func TestExtractAllModifiedFiles_SubagentOnlyChanges(t *testing.T) { + t.Parallel() + + tmpDir := t.TempDir() + transcriptPath := tmpDir + "/transcript.jsonl" + subagentsDir := tmpDir + "/tasks/toolu_task1" + + if err := os.MkdirAll(subagentsDir, 0o755); err != nil { + t.Fatalf("failed to create subagents dir: %v", err) + } + + // Main transcript: ONLY a Task call, no direct file modifications + // This is the key bug scenario - if we only look at the main transcript, + // we miss all the subagent's file changes entirely. + writeJSONLFile(t, transcriptPath, + makeTaskToolUseLine(t, "a1", "toolu_task1"), + makeTaskResultLine(t, "u1", "toolu_task1", "sub1"), + ) + + // Subagent transcript: Write to two files + writeJSONLFile(t, subagentsDir+"/agent-sub1.jsonl", + makeWriteToolLine(t, "sa1", "/repo/subagent_file1.go"), + makeWriteToolLine(t, "sa2", "/repo/subagent_file2.go"), + ) + + files, err := ExtractAllModifiedFiles(transcriptPath, 0, subagentsDir) + if err != nil { + t.Fatalf("ExtractAllModifiedFiles() error: %v", err) + } + + if len(files) != 2 { + t.Errorf("expected 2 files from subagent, got %d: %v", len(files), files) + } + + wantFiles := map[string]bool{ + "/repo/subagent_file1.go": true, + "/repo/subagent_file2.go": true, + } + for _, f := range files { + if !wantFiles[f] { + t.Errorf("unexpected file %q in result", f) + } + delete(wantFiles, f) + } + for f := range wantFiles { + t.Errorf("missing expected file %q", f) + } +} diff --git a/cmd/entire/cli/hooks_claudecode_handlers.go b/cmd/entire/cli/hooks_claudecode_handlers.go index d496d09e8..b2fade179 100644 --- a/cmd/entire/cli/hooks_claudecode_handlers.go +++ b/cmd/entire/cli/hooks_claudecode_handlers.go @@ -221,8 +221,15 @@ func commitWithMetadata() error { //nolint:maintidx // already present in codeba } fmt.Fprintf(os.Stderr, "Extracted summary to: %s\n", sessionDir+"/"+paths.SummaryFileName) - // Get modified files from transcript - modifiedFiles := extractModifiedFiles(transcript) + // Get modified files from transcript (main agent + subagents). + // Subagent transcripts live in //subagents/ + subagentsDir := filepath.Join(filepath.Dir(transcriptPath), input.SessionID, "subagents") + modifiedFiles, subagentErr := claudecode.ExtractAllModifiedFiles(transcriptPath, transcriptOffset, subagentsDir) + if subagentErr != nil { + fmt.Fprintf(os.Stderr, "Warning: failed to extract modified files with subagents: %v\n", subagentErr) + // Fall back to main transcript only + modifiedFiles = extractModifiedFiles(transcript) + } // Generate commit message from last user prompt lastPrompt := "" @@ -321,11 +328,9 @@ func commitWithMetadata() error { //nolint:maintidx // already present in codeba // Calculate token usage for this checkpoint (Claude Code specific) var tokenUsage *agent.TokenUsage if transcriptPath != "" { - // Subagents are stored in a subagents/ directory next to the main transcript - subagentsDir := filepath.Join(filepath.Dir(transcriptPath), sessionID, "subagents") - usage, err := claudecode.CalculateTotalTokenUsage(transcriptPath, transcriptLinesAtStart, subagentsDir) - if err != nil { - fmt.Fprintf(os.Stderr, "Warning: failed to calculate token usage: %v\n", err) + usage, tokenErr := claudecode.CalculateTotalTokenUsage(transcriptPath, transcriptLinesAtStart, subagentsDir) + if tokenErr != nil { + fmt.Fprintf(os.Stderr, "Warning: failed to calculate token usage: %v\n", tokenErr) } else { tokenUsage = usage } diff --git a/cmd/entire/cli/integration_test/hooks_test.go b/cmd/entire/cli/integration_test/hooks_test.go index b38c2ffe6..6e3a568bc 100644 --- a/cmd/entire/cli/integration_test/hooks_test.go +++ b/cmd/entire/cli/integration_test/hooks_test.go @@ -125,6 +125,106 @@ func TestSession_CreateTranscript(t *testing.T) { }) } +// TestHookRunner_SimulateStop_SubagentOnlyChanges tests that the stop hook correctly +// creates a checkpoint when ONLY subagents (spawned via Task tool) modify files. +// This is a regression test for ENT-297 where subagent file modifications were invisible +// to checkpoint detection because ExtractModifiedFiles only looked at the main transcript. +// The fix (ExtractAllModifiedFiles) also reads subagent transcript files. +func TestHookRunner_SimulateStop_SubagentOnlyChanges(t *testing.T) { + t.Parallel() + RunForAllStrategiesWithRepoEnv(t, func(t *testing.T, env *TestEnv, strategyName string) { + // Create a session + session := env.NewSession() + + // Simulate user prompt submit first (captures pre-prompt state) + err := env.SimulateUserPromptSubmit(session.ID) + if err != nil { + t.Fatalf("SimulateUserPromptSubmit failed: %v", err) + } + + // Record initial state for comparison + commitsBefore := env.GetGitLog() + + // Create a file on disk (simulating what a subagent would write) + env.WriteFile("subagent_output.go", "package main\n\nfunc SubagentWork() {}\n") + + // Build the main transcript manually. The main transcript contains ONLY + // a Task tool call (no Write/Edit). All file modifications happened in + // the subagent. + mainTranscript := NewTranscriptBuilder() + mainTranscript.AddUserMessage("Create a function in a new file") + mainTranscript.AddAssistantMessage("I'll delegate this to a subagent.") + + // Add Task tool use + taskToolUseID := mainTranscript.AddTaskToolUse("", "Create the function") + + // Add Task tool result with agentId + agentID := "sub123abc" + mainTranscript.AddTaskToolResult(taskToolUseID, agentID) + + mainTranscript.AddAssistantMessage("The subagent completed the task.") + + // Write the main transcript + if err := mainTranscript.WriteToFile(session.TranscriptPath); err != nil { + t.Fatalf("failed to write main transcript: %v", err) + } + + // Create the subagent transcript file in the directory structure the Stop hook expects: + // //subagents/agent-.jsonl + transcriptDir := filepath.Dir(session.TranscriptPath) + subagentsDir := filepath.Join(transcriptDir, session.ID, "subagents") + subagentTranscriptPath := filepath.Join(subagentsDir, "agent-"+agentID+".jsonl") + + // Build the subagent transcript with Write tool calls + subagentTranscript := NewTranscriptBuilder() + subagentTranscript.AddUserMessage("Create a function in a new file") + absFilePath := filepath.Join(env.RepoDir, "subagent_output.go") + toolID := subagentTranscript.AddToolUse("Write", absFilePath, "package main\n\nfunc SubagentWork() {}\n") + subagentTranscript.AddToolResult(toolID) + subagentTranscript.AddAssistantMessage("Done, I created the function.") + + if err := subagentTranscript.WriteToFile(subagentTranscriptPath); err != nil { + t.Fatalf("failed to write subagent transcript: %v", err) + } + + // Simulate stop - this should NOT error and should create a checkpoint + err = env.SimulateStop(session.ID, session.TranscriptPath) + if err != nil { + t.Fatalf("SimulateStop failed: %v", err) + } + + // Verify checkpoint was created based on strategy type + switch strategyName { + case strategy.StrategyNameAutoCommit: + // Auto-commit creates a new commit on the active branch + commitsAfter := env.GetGitLog() + if len(commitsAfter) <= len(commitsBefore) { + t.Errorf("auto-commit: expected new commit to be created; commits before=%d, after=%d", + len(commitsBefore), len(commitsAfter)) + } + + case strategy.StrategyNameManualCommit: + // Manual-commit stores checkpoint data on the shadow branch + shadowBranch := env.GetShadowBranchName() + if !env.BranchExists(shadowBranch) { + t.Errorf("manual-commit: shadow branch %s should exist after checkpoint", shadowBranch) + } + + // Verify session state was updated with checkpoint count + state, stateErr := env.GetSessionState(session.ID) + if stateErr != nil { + t.Fatalf("failed to get session state: %v", stateErr) + } + if state == nil { + t.Fatal("manual-commit: session state should exist after checkpoint") + } + if state.StepCount == 0 { + t.Error("manual-commit: session state should have non-zero step count") + } + } + }) +} + // TestUserPromptSubmit_ReinstallsOverwrittenHooks verifies that EnsureSetup is called // during user-prompt-submit (start of turn) and reinstalls hooks that were overwritten // by third-party tools like lefthook. This ensures hooks are in place before any diff --git a/cmd/entire/cli/integration_test/transcript.go b/cmd/entire/cli/integration_test/transcript.go index 52f5a7d64..b205a270a 100644 --- a/cmd/entire/cli/integration_test/transcript.go +++ b/cmd/entire/cli/integration_test/transcript.go @@ -140,7 +140,7 @@ func (b *TranscriptBuilder) AddTaskToolResult(toolUseID, agentID string) string { "type": "tool_result", "tool_use_id": toolUseID, - "content": "Task completed by agent " + agentID, + "content": "agentId: " + agentID, }, }, }, diff --git a/cmd/entire/cli/strategy/auto_commit.go b/cmd/entire/cli/strategy/auto_commit.go index 29216a15e..c0843bd00 100644 --- a/cmd/entire/cli/strategy/auto_commit.go +++ b/cmd/entire/cli/strategy/auto_commit.go @@ -84,7 +84,7 @@ func (s *AutoCommitStrategy) getCheckpointStore() (*checkpoint.GitStore, error) // NewAutoCommitStrategy creates a new AutoCommitStrategy instance // -func NewAutoCommitStrategy() Strategy { //nolint:ireturn // already present in codebase +func NewAutoCommitStrategy() Strategy { //nolint:ireturn // factory returns interface by design return &AutoCommitStrategy{} } diff --git a/cmd/entire/cli/strategy/manual_commit.go b/cmd/entire/cli/strategy/manual_commit.go index ffc094265..2204db0f2 100644 --- a/cmd/entire/cli/strategy/manual_commit.go +++ b/cmd/entire/cli/strategy/manual_commit.go @@ -58,7 +58,7 @@ func (s *ManualCommitStrategy) getCheckpointStore() (*checkpoint.GitStore, error // NewManualCommitStrategy creates a new manual-commit strategy instance. // -func NewManualCommitStrategy() Strategy { //nolint:ireturn // already present in codebase +func NewManualCommitStrategy() Strategy { //nolint:ireturn // factory returns interface by design return &ManualCommitStrategy{} } @@ -66,7 +66,7 @@ func NewManualCommitStrategy() Strategy { //nolint:ireturn // already present in // This legacy constructor delegates to NewManualCommitStrategy. // -func NewShadowStrategy() Strategy { //nolint:ireturn // already present in codebase +func NewShadowStrategy() Strategy { //nolint:ireturn // factory returns interface by design return NewManualCommitStrategy() } diff --git a/cmd/entire/cli/strategy/manual_commit_condensation.go b/cmd/entire/cli/strategy/manual_commit_condensation.go index 00bf7535e..bb797e558 100644 --- a/cmd/entire/cli/strategy/manual_commit_condensation.go +++ b/cmd/entire/cli/strategy/manual_commit_condensation.go @@ -436,34 +436,8 @@ func (s *ManualCommitStrategy) extractSessionDataFromLiveTranscript(state *Sessi if len(state.FilesTouched) > 0 { data.FilesTouched = state.FilesTouched } else { - // Extract modified files from transcript - ag, agErr := agent.GetByAgentType(state.AgentType) - if agErr == nil { - if analyzer, ok := ag.(agent.TranscriptAnalyzer); ok { - modifiedFiles, _, extractErr := analyzer.ExtractModifiedFilesFromOffset(state.TranscriptPath, state.CheckpointTranscriptStart) - if extractErr == nil && len(modifiedFiles) > 0 { - // Normalize to repo-relative paths - basePath := state.WorktreePath - if basePath == "" { - if wp, wpErr := GetWorktreePath(); wpErr == nil { - basePath = wp - } - } - if basePath != "" { - normalized := make([]string, 0, len(modifiedFiles)) - for _, f := range modifiedFiles { - if rel := paths.ToRelativePath(f, basePath); rel != "" { - normalized = append(normalized, rel) - } else { - normalized = append(normalized, f) - } - } - modifiedFiles = normalized - } - data.FilesTouched = modifiedFiles - } - } - } + // Use the shared helper which includes subagent transcripts + data.FilesTouched = s.extractModifiedFilesFromLiveTranscript(state, state.CheckpointTranscriptStart) } // Calculate token usage from the extracted transcript portion diff --git a/cmd/entire/cli/strategy/manual_commit_hooks.go b/cmd/entire/cli/strategy/manual_commit_hooks.go index c416fd5e9..f4c9a4aad 100644 --- a/cmd/entire/cli/strategy/manual_commit_hooks.go +++ b/cmd/entire/cli/strategy/manual_commit_hooks.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/entireio/cli/cmd/entire/cli/agent" + "github.com/entireio/cli/cmd/entire/cli/agent/claudecode" "github.com/entireio/cli/cmd/entire/cli/checkpoint" "github.com/entireio/cli/cmd/entire/cli/checkpoint/id" @@ -536,23 +537,15 @@ func (s *ManualCommitStrategy) PostCommit() error { // // For ACTIVE sessions: the commit has a checkpoint trailer (verified above), // meaning PrepareCommitMsg already determined this commit is session-related. - // We trust that and assume hasNew = true, bypassing sessionHasNewContent which - // would incorrectly return false (uses getStagedFiles, but files are no longer - // staged after the commit). + // The trailer is only added when either: + // - No TTY (agent/subagent committing) — added unconditionally + // - TTY (human committing) — added after content detection confirmed agent work + // In both cases, PrepareCommitMsg already validated this commit. We trust + // that decision here. Transcript-based re-validation is unreliable because + // subagent transcripts may not be available yet (subagent still running). var hasNew bool if state.Phase.IsActive() { - // For ACTIVE sessions, check if this session has any content to condense. - // A session with no checkpoints (StepCount=0) and no files touched may exist - // concurrently with other sessions that DO have content. - // We only condense if this session actually has work. - if state.StepCount > 0 || len(state.FilesTouched) > 0 { - hasNew = true - } else { - // No checkpoints and no tracked files - check the live transcript. - // Use sessionHasNewContentInCommittedFiles because staged files are empty - // after the commit (files have already been committed). - hasNew = s.sessionHasNewContentInCommittedFiles(state, committedFileSet) - } + hasNew = true } else { var contentErr error hasNew, contentErr = s.sessionHasNewContent(repo, state) @@ -944,81 +937,11 @@ func (s *ManualCommitStrategy) sessionHasNewContent(repo *git.Repository, state func (s *ManualCommitStrategy) sessionHasNewContentFromLiveTranscript(repo *git.Repository, state *SessionState) (bool, error) { logCtx := logging.WithComponent(context.Background(), "checkpoint") - // Need both transcript path and agent type to analyze - if state.TranscriptPath == "" || state.AgentType == "" { - logging.Debug(logCtx, "live transcript check: missing transcript path or agent type", - slog.String("session_id", state.SessionID), - slog.String("transcript_path", state.TranscriptPath), - slog.String("agent_type", string(state.AgentType)), - ) + modifiedFiles, ok := s.extractNewModifiedFilesFromLiveTranscript(state) + if !ok || len(modifiedFiles) == 0 { return false, nil } - // Get the agent for transcript analysis - ag, err := agent.GetByAgentType(state.AgentType) - if err != nil { - return false, nil //nolint:nilerr // Unknown agent type, fail gracefully - } - - // Cast to TranscriptAnalyzer - analyzer, ok := ag.(agent.TranscriptAnalyzer) - if !ok { - return false, nil // Agent doesn't support transcript analysis - } - - // Get current transcript position - currentPos, err := analyzer.GetTranscriptPosition(state.TranscriptPath) - if err != nil { - return false, nil //nolint:nilerr // Error reading transcript, fail gracefully - } - - // Check if transcript has grown since last condensation - if currentPos <= state.CheckpointTranscriptStart { - logging.Debug(logCtx, "live transcript check: no new content", - slog.String("session_id", state.SessionID), - slog.Int("current_pos", currentPos), - slog.Int("start_offset", state.CheckpointTranscriptStart), - ) - return false, nil // No new content - } - - // Transcript has grown - check if there are file modifications in the new portion - modifiedFiles, _, err := analyzer.ExtractModifiedFilesFromOffset(state.TranscriptPath, state.CheckpointTranscriptStart) - if err != nil { - return false, nil //nolint:nilerr // Error parsing transcript, fail gracefully - } - - // No file modifications means no new content to checkpoint - if len(modifiedFiles) == 0 { - logging.Debug(logCtx, "live transcript check: transcript grew but no file modifications", - slog.String("session_id", state.SessionID), - ) - return false, nil - } - - // Normalize modified files from absolute to repo-relative paths. - // Transcript tool_use entries contain absolute paths (e.g., /Users/alex/project/src/main.go) - // but getStagedFiles returns repo-relative paths (e.g., src/main.go). - // Use state.WorktreePath (already resolved) to avoid an extra git subprocess. - basePath := state.WorktreePath - if basePath == "" { - if wp, wpErr := GetWorktreePath(); wpErr == nil { - basePath = wp - } - } - if basePath != "" { - normalized := make([]string, 0, len(modifiedFiles)) - for _, f := range modifiedFiles { - if rel := paths.ToRelativePath(f, basePath); rel != "" { - normalized = append(normalized, rel) - } else { - // Already relative or outside repo — keep as-is - normalized = append(normalized, f) - } - } - modifiedFiles = normalized - } - logging.Debug(logCtx, "live transcript check: found file modifications", slog.String("session_id", state.SessionID), slog.Int("modified_files", len(modifiedFiles)), @@ -1045,162 +968,106 @@ func (s *ManualCommitStrategy) sessionHasNewContentFromLiveTranscript(repo *git. return true, nil } -// sessionHasNewContentInCommittedFiles checks if a session has content that overlaps with -// the committed files. This is used in PostCommit for ACTIVE sessions where staged files -// are empty (already committed). Uses the live transcript to extract modified files and -// compares against the committed file set. -// -// KNOWN LIMITATION: This function relies on transcript analysis to detect file modifications. -// If the agent makes file modifications via shell commands (e.g., `sed`, `mv`, `cp`) that -// aren't captured in the transcript's file modification tracking, those modifications may -// not be detected. This is an acceptable edge case because: -// 1. Most agent file modifications use the Write/Edit tools which are tracked -// 2. Shell-based modifications are relatively rare in practice -// 3. The consequence (missing a checkpoint trailer) is minor - the transcript is still saved -func (s *ManualCommitStrategy) sessionHasNewContentInCommittedFiles(state *SessionState, committedFiles map[string]struct{}) bool { +// extractFilesFromLiveTranscript extracts modified file paths from the live transcript. +// Returns empty slice if extraction fails (fail-open behavior for hooks). +// Extracts ALL files from the transcript (offset 0) because this is used for carry-forward +// computation which needs to know all files touched, not just new ones. +func (s *ManualCommitStrategy) extractFilesFromLiveTranscript(state *SessionState) []string { + return s.extractModifiedFilesFromLiveTranscript(state, 0) +} + +// extractNewModifiedFilesFromLiveTranscript extracts modified files from the live +// transcript that are NEW since the last condensation. Returns the normalized file list +// and whether the extraction succeeded. Used by sessionHasNewContentFromLiveTranscript +// to detect agent work. +func (s *ManualCommitStrategy) extractNewModifiedFilesFromLiveTranscript(state *SessionState) ([]string, bool) { logCtx := logging.WithComponent(context.Background(), "checkpoint") - // Need both transcript path and agent type to analyze if state.TranscriptPath == "" || state.AgentType == "" { - logging.Debug(logCtx, "committed files check: missing transcript path or agent type", - slog.String("session_id", state.SessionID), - slog.String("transcript_path", state.TranscriptPath), - slog.String("agent_type", string(state.AgentType)), - ) - return false + return nil, false } - // Get the agent for transcript analysis ag, err := agent.GetByAgentType(state.AgentType) if err != nil { - return false // Unknown agent type, fail gracefully + return nil, false } - // Cast to TranscriptAnalyzer analyzer, ok := ag.(agent.TranscriptAnalyzer) if !ok { - return false // Agent doesn't support transcript analysis + return nil, false } - // Get current transcript position + // Check if transcript has grown since last condensation currentPos, err := analyzer.GetTranscriptPosition(state.TranscriptPath) if err != nil { - return false // Error reading transcript, fail gracefully + return nil, false } - - // Check if transcript has grown since last condensation if currentPos <= state.CheckpointTranscriptStart { - logging.Debug(logCtx, "committed files check: no new content", + logging.Debug(logCtx, "live transcript check: no new content", slog.String("session_id", state.SessionID), slog.Int("current_pos", currentPos), slog.Int("start_offset", state.CheckpointTranscriptStart), ) - return false // No new content - } - - // Transcript has grown - check if there are file modifications in the new portion - modifiedFiles, _, err := analyzer.ExtractModifiedFilesFromOffset(state.TranscriptPath, state.CheckpointTranscriptStart) - if err != nil { - return false // Error parsing transcript, fail gracefully - } - - // No file modifications means no new content to checkpoint - if len(modifiedFiles) == 0 { - logging.Debug(logCtx, "committed files check: transcript grew but no file modifications", - slog.String("session_id", state.SessionID), - ) - return false + return nil, true // No new content, but extraction succeeded } - // Normalize modified files from absolute to repo-relative paths. - basePath := state.WorktreePath - if basePath == "" { - if wp, wpErr := GetWorktreePath(); wpErr == nil { - basePath = wp - } - } - if basePath != "" { - normalized := make([]string, 0, len(modifiedFiles)) - for _, f := range modifiedFiles { - if rel := paths.ToRelativePath(f, basePath); rel != "" { - normalized = append(normalized, rel) - } else { - normalized = append(normalized, f) - } - } - modifiedFiles = normalized - } - - logging.Debug(logCtx, "committed files check: found file modifications", - slog.String("session_id", state.SessionID), - slog.Int("modified_files", len(modifiedFiles)), - slog.Int("committed_files", len(committedFiles)), - ) - - // Check if any modified files overlap with committed files - for _, f := range modifiedFiles { - if _, ok := committedFiles[f]; ok { - return true - } - } - - logging.Debug(logCtx, "committed files check: no overlap between committed and modified files", - slog.String("session_id", state.SessionID), - ) - return false + return s.extractModifiedFilesFromLiveTranscript(state, state.CheckpointTranscriptStart), true } -// extractFilesFromLiveTranscript extracts modified file paths from the live transcript. -// Returns empty slice if extraction fails (fail-open behavior for hooks). -// Extracts ALL files from the transcript (offset 0) because this is used for carry-forward -// computation which needs to know all files touched, not just new ones. -func (s *ManualCommitStrategy) extractFilesFromLiveTranscript(state *SessionState) []string { +// extractModifiedFilesFromLiveTranscript extracts modified files from the live transcript +// (including subagent transcripts) starting from the given offset, and normalizes them +// to repo-relative paths. Returns the normalized file list. +func (s *ManualCommitStrategy) extractModifiedFilesFromLiveTranscript(state *SessionState, offset int) []string { logCtx := logging.WithComponent(context.Background(), "checkpoint") if state.TranscriptPath == "" || state.AgentType == "" { - logging.Debug(logCtx, "extractFilesFromLiveTranscript: missing path or agent type", - slog.String("transcript_path", state.TranscriptPath), - slog.String("agent_type", string(state.AgentType)), - ) return nil } ag, err := agent.GetByAgentType(state.AgentType) if err != nil { - logging.Debug(logCtx, "extractFilesFromLiveTranscript: agent not found", - slog.String("agent_type", string(state.AgentType)), - slog.String("error", err.Error()), - ) return nil } analyzer, ok := ag.(agent.TranscriptAnalyzer) if !ok { - logging.Debug(logCtx, "extractFilesFromLiveTranscript: agent is not a TranscriptAnalyzer", - slog.String("agent_type", string(state.AgentType)), - ) return nil } - // Extract ALL files from transcript (offset 0) for carry-forward computation. - // state.CheckpointTranscriptStart may already be updated after condensation, - // but carry-forward needs to know all files touched to compute remaining files. - modifiedFiles, _, err := analyzer.ExtractModifiedFilesFromOffset(state.TranscriptPath, 0) - if err != nil || len(modifiedFiles) == 0 { - logging.Debug(logCtx, "extractFilesFromLiveTranscript: no files extracted", - slog.String("transcript_path", state.TranscriptPath), - slog.Int("files_count", len(modifiedFiles)), - slog.Any("error", err), - ) - return nil + var modifiedFiles []string + + // For Claude Code, use ExtractAllModifiedFiles which parses the main transcript + // AND subagent transcripts in a single pass, avoiding redundant parsing. + if state.AgentType == agent.AgentTypeClaudeCode { + subagentsDir := filepath.Join(filepath.Dir(state.TranscriptPath), state.SessionID, "subagents") + allFiles, extractErr := claudecode.ExtractAllModifiedFiles(state.TranscriptPath, offset, subagentsDir) + if extractErr != nil { + logging.Debug(logCtx, "extractModifiedFilesFromLiveTranscript: extraction failed", + slog.String("session_id", state.SessionID), + slog.String("error", extractErr.Error()), + ) + } else { + modifiedFiles = allFiles + } + } else { + files, _, err := analyzer.ExtractModifiedFilesFromOffset(state.TranscriptPath, offset) + if err != nil { + logging.Debug(logCtx, "extractModifiedFilesFromLiveTranscript: main transcript extraction failed", + slog.String("transcript_path", state.TranscriptPath), + slog.Any("error", err), + ) + } else { + modifiedFiles = files + } } - logging.Debug(logCtx, "extractFilesFromLiveTranscript: files extracted", - slog.Int("files_count", len(modifiedFiles)), - slog.Any("files", modifiedFiles), - ) + if len(modifiedFiles) == 0 { + return nil + } - // Normalize to repo-relative paths + // Normalize to repo-relative paths. + // Transcript tool_use entries contain absolute paths (e.g., /Users/alex/project/src/main.go) + // but getStagedFiles/committedFiles use repo-relative paths (e.g., src/main.go). basePath := state.WorktreePath if basePath == "" { if wp, wpErr := GetWorktreePath(); wpErr == nil { diff --git a/cmd/entire/cli/strategy/mid_turn_commit_test.go b/cmd/entire/cli/strategy/mid_turn_commit_test.go index 1b8eb2d1c..f6cda77a7 100644 --- a/cmd/entire/cli/strategy/mid_turn_commit_test.go +++ b/cmd/entire/cli/strategy/mid_turn_commit_test.go @@ -91,6 +91,97 @@ func TestSessionHasNewContentFromLiveTranscript_NormalizesAbsolutePaths(t *testi "that match repo-relative staged files after normalization") } +// TestSessionHasNewContentFromLiveTranscript_IncludesSubagentFiles verifies +// that sessionHasNewContentFromLiveTranscript detects file modifications made by +// subagents spawned via the Task tool. +// +// Bug: The function only scans the main transcript via ExtractModifiedFilesFromOffset, +// which misses file changes made by subagents. When the main transcript contains only +// a Task tool call (no direct Write/Edit), modifiedFiles is empty and the function +// returns false -- even though the subagent wrote files that are staged for commit. +func TestSessionHasNewContentFromLiveTranscript_IncludesSubagentFiles(t *testing.T) { + dir := setupGitRepo(t) + t.Chdir(dir) + + repo, err := git.PlainOpen(dir) + require.NoError(t, err) + + s := &ManualCommitStrategy{} + + // Create a file that we'll stage + srcDir := filepath.Join(dir, "src") + require.NoError(t, os.MkdirAll(srcDir, 0o755)) + testFile := filepath.Join(srcDir, "feature.go") + require.NoError(t, os.WriteFile(testFile, []byte("package src\n"), 0o644)) + + wt, err := repo.Worktree() + require.NoError(t, err) + _, err = wt.Add("src/feature.go") + require.NoError(t, err) + + // Get the resolved worktree path first — on macOS, t.TempDir() returns /var/... + // but git resolves symlinks to /private/var/... . Claude Code uses the resolved + // path in its transcript, so we must too. + worktreePath, err := GetWorktreePath() + require.NoError(t, err) + worktreeID, err := paths.GetWorktreeID(worktreePath) + require.NoError(t, err) + + // Create a main transcript that ONLY has a Task tool call — no direct Write/Edit. + // The assistant invokes Task, and the user line returns the tool_result with agentId. + const modelSessionID = "model-session-sub" + transcriptDir := filepath.Join(dir, ".entire", "metadata") + require.NoError(t, os.MkdirAll(transcriptDir, 0o755)) + + mainTranscript := `{"type":"assistant","uuid":"a1","message":{"content":[{"type":"tool_use","id":"toolu_task1","name":"Task","input":{"prompt":"implement feature"}}]}} +{"type":"user","uuid":"u1","message":{"content":[{"type":"tool_result","tool_use_id":"toolu_task1","content":"agentId: sub123"}]}} +` + transcriptPath := filepath.Join(transcriptDir, "transcript.jsonl") + require.NoError(t, os.WriteFile(transcriptPath, []byte(mainTranscript), 0o644)) + + // Create the subagent transcript with a Write tool_use targeting the staged file. + // Path: //subagents/agent-sub123.jsonl + absFeaturePath := filepath.Join(worktreePath, "src", "feature.go") + subagentsDir := filepath.Join(transcriptDir, modelSessionID, "subagents") + require.NoError(t, os.MkdirAll(subagentsDir, 0o755)) + + subagentTranscript := `{"type":"assistant","uuid":"sa1","message":{"content":[{"type":"tool_use","id":"toolu_write1","name":"Write","input":{"file_path":"` + absFeaturePath + `","content":"package src\n"}}]}} +` + require.NoError(t, os.WriteFile( + filepath.Join(subagentsDir, "agent-sub123.jsonl"), + []byte(subagentTranscript), 0o644, + )) + + // Create session state: no shadow branch, transcript has only Task calls, + // agent type is Claude Code so the subagent path resolution works + now := time.Now() + head, err := repo.Head() + require.NoError(t, err) + + state := &SessionState{ + SessionID: modelSessionID, + BaseCommit: head.Hash().String(), + WorktreePath: worktreePath, + WorktreeID: worktreeID, + StartedAt: now, + Phase: session.PhaseActive, + LastInteractionTime: &now, + AgentType: agent.AgentTypeClaudeCode, + TranscriptPath: transcriptPath, + CheckpointTranscriptStart: 0, + } + require.NoError(t, s.saveSessionState(state)) + + // Call sessionHasNewContent — should fall through to live transcript check + // since there's no shadow branch, and should detect subagent file modifications + hasNew, err := s.sessionHasNewContent(repo, state) + require.NoError(t, err) + assert.True(t, hasNew, + "sessionHasNewContent should return true when subagent transcript "+ + "contains Write tool calls for staged files, even though the main "+ + "transcript has no direct file modifications") +} + // TestPostCommit_NoTrailer_UpdatesBaseCommit verifies that when a commit has no // Entire-Checkpoint trailer, PostCommit still updates BaseCommit for active sessions. // diff --git a/cmd/entire/cli/strategy/phase_postcommit_test.go b/cmd/entire/cli/strategy/phase_postcommit_test.go index 331a34a8a..d321ca34f 100644 --- a/cmd/entire/cli/strategy/phase_postcommit_test.go +++ b/cmd/entire/cli/strategy/phase_postcommit_test.go @@ -172,12 +172,12 @@ func TestPostCommit_RebaseDuringActive_SkipsTransition(t *testing.T) { "shadow branch should be preserved during rebase") } -// TestPostCommit_ShadowBranch_PreservedWhenUncondensedActiveSessionExists verifies that -// the shadow branch is preserved when an ACTIVE session that was NOT condensed -// (e.g., it has no new content) still shares the branch with another session. -// Both sessions condense immediately on GitCommit, but the branch is only deleted -// when ALL sessions on it have been condensed. -func TestPostCommit_ShadowBranch_PreservedWhenUncondensedActiveSessionExists(t *testing.T) { +// TestPostCommit_ActiveSessionAlwaysCondenses verifies that an ACTIVE session +// is always condensed on GitCommit, even when it has no checkpoints or tracked files. +// This is because PrepareCommitMsg already validated the trailer, so PostCommit +// trusts that decision rather than re-validating via transcript analysis (which is +// unreliable when subagents are still running). +func TestPostCommit_ActiveSessionAlwaysCondenses(t *testing.T) { dir := setupGitRepo(t) t.Chdir(dir) @@ -206,8 +206,8 @@ func TestPostCommit_ShadowBranch_PreservedWhenUncondensedActiveSessionExists(t * // Create a second session with the SAME base commit and worktree (concurrent session). // This session is ACTIVE but has NO checkpoints (StepCount=0, no shadow branch content). - // Because it has no new content, it will NOT be condensed, and its shadow branch must - // be preserved so it can save checkpoints later. + // Despite having no content, it WILL be condensed because ACTIVE sessions always + // condense — PrepareCommitMsg already validated the trailer. now := time.Now() activeState := &SessionState{ SessionID: activeSessionID, @@ -237,23 +237,23 @@ func TestPostCommit_ShadowBranch_PreservedWhenUncondensedActiveSessionExists(t * assert.Equal(t, session.PhaseActive, activeState.Phase, "ACTIVE session should stay ACTIVE after GitCommit") - // Verify the IDLE session actually condensed (entire/checkpoints/v1 branch should exist) + // Verify both sessions 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 preserved because the ACTIVE session was not condensed - // (it had no new content) and still needs the branch for future checkpoints. + // Verify shadow branch is cleaned up because ALL sessions condensed + // (both IDLE and ACTIVE were condensed on this commit) refName := plumbing.NewBranchReferenceName(shadowBranch) _, err = repo.Reference(refName, true) - assert.NoError(t, err, - "shadow branch should be preserved when an uncondensed active session still exists on it") + assert.Error(t, err, + "shadow branch should be deleted when all sessions have been condensed") } // TestPostCommit_CondensationFailure_PreservesShadowBranch verifies that when diff --git a/cmd/entire/cli/strategy/registry.go b/cmd/entire/cli/strategy/registry.go index a57842427..bdde3069c 100644 --- a/cmd/entire/cli/strategy/registry.go +++ b/cmd/entire/cli/strategy/registry.go @@ -24,7 +24,7 @@ func Register(name string, factory Factory) { // Get retrieves a strategy by name. // Returns an error if the strategy is not registered. -func Get(name string) (Strategy, error) { //nolint:ireturn // already present in codebase +func Get(name string) (Strategy, error) { //nolint:ireturn // registry returns interface by design registryMu.RLock() defer registryMu.RUnlock() @@ -61,7 +61,7 @@ const DefaultStrategyName = StrategyNameManualCommit // Default returns the default strategy. // Falls back to returning nil if no strategies are registered. -func Default() Strategy { //nolint:ireturn // already present in codebase +func Default() Strategy { //nolint:ireturn // registry returns interface by design s, err := Get(DefaultStrategyName) if err != nil { // Fallback: return the first registered strategy