diff --git a/pkg/cli/add_command.go b/pkg/cli/add_command.go index 85adfc43f1d..afd5ef1ffad 100644 --- a/pkg/cli/add_command.go +++ b/pkg/cli/add_command.go @@ -240,14 +240,7 @@ func AddResolvedWorkflows(workflowStrings []string, resolved *ResolvedWorkflows, func addWorkflows(workflows []*ResolvedWorkflow, opts AddOptions) error { addLog.Printf("Adding %d workflow(s) to repository", len(workflows)) // Create file tracker for all operations - tracker, err := NewFileTracker() - if err != nil { - // If we can't create a tracker (e.g., not in git repo), fall back to non-tracking behavior - if opts.Verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Could not create file tracker: %v", err))) - } - tracker = nil - } + tracker := NewFileTracker() return addWorkflowsWithTracking(workflows, tracker, opts) } diff --git a/pkg/cli/add_workflow_pr.go b/pkg/cli/add_workflow_pr.go index 54df1f6adda..5d37eda45a4 100644 --- a/pkg/cli/add_workflow_pr.go +++ b/pkg/cli/add_workflow_pr.go @@ -70,10 +70,7 @@ func addWorkflowsWithPR(workflows []*ResolvedWorkflow, opts AddOptions) (int, st } // Create file tracker for rollback capability - tracker, err := NewFileTracker() - if err != nil { - return 0, "", fmt.Errorf("failed to create file tracker: %w", err) - } + tracker := NewFileTracker() // Ensure we switch back to original branch on exit defer func() { diff --git a/pkg/cli/file_tracker.go b/pkg/cli/file_tracker.go index e03114fe7a4..15aebf1e02f 100644 --- a/pkg/cli/file_tracker.go +++ b/pkg/cli/file_tracker.go @@ -24,18 +24,27 @@ type FileTracker struct { } // NewFileTracker creates a new file tracker -func NewFileTracker() (*FileTracker, error) { +func NewFileTracker() *FileTracker { fileTrackerLog.Print("Creating new file tracker") + return &FileTracker{ + OriginalContent: make(map[string][]byte), + } +} + +func (ft *FileTracker) ensureGitRoot() error { + if ft.gitRoot != "" { + return nil + } + gitRoot, err := gitutil.FindGitRoot() if err != nil { fileTrackerLog.Printf("Failed to find git root: %v", err) - return nil, fmt.Errorf("file tracker requires being in a git repository: %w", err) + return fmt.Errorf("file tracker requires being in a git repository: %w", err) } + + ft.gitRoot = gitRoot fileTrackerLog.Printf("File tracker initialized with git root: %s", gitRoot) - return &FileTracker{ - OriginalContent: make(map[string][]byte), - gitRoot: gitRoot, - }, nil + return nil } // TrackCreated adds a file to the created files list @@ -95,6 +104,9 @@ func (ft *FileTracker) StageAllFiles(verbose bool) error { } // Stage all files in a single git add command + if err := ft.ensureGitRoot(); err != nil { + return err + } args := append([]string{"add"}, allFiles...) cmd := exec.Command("git", args...) cmd.Dir = ft.gitRoot diff --git a/pkg/cli/file_tracker_test.go b/pkg/cli/file_tracker_test.go index 0eb7431802f..9cee77a951b 100644 --- a/pkg/cli/file_tracker_test.go +++ b/pkg/cli/file_tracker_test.go @@ -7,6 +7,7 @@ import ( "os/exec" "path/filepath" "slices" + "strings" "testing" ) @@ -34,10 +35,7 @@ func TestFileTracker_CreationAndTracking(t *testing.T) { } // Create file tracker - tracker, err := NewFileTracker() - if err != nil { - t.Fatalf("Failed to create file tracker: %v", err) - } + tracker := NewFileTracker() // Create test files testFile1 := filepath.Join(tempDir, "test1.md") @@ -106,10 +104,7 @@ func TestFileTracker_ModifiedFiles(t *testing.T) { } // Create file tracker - tracker, err := NewFileTracker() - if err != nil { - t.Fatalf("Failed to create file tracker: %v", err) - } + tracker := NewFileTracker() // Create existing file testFile := filepath.Join(tempDir, "existing.md") @@ -211,10 +206,7 @@ func TestFileTracker_RollbackAllFiles(t *testing.T) { } // Create file tracker - tracker, err := NewFileTracker() - if err != nil { - t.Fatalf("Failed to create file tracker: %v", err) - } + tracker := NewFileTracker() // Create an existing file existingFile := filepath.Join(tempDir, "existing.md") @@ -301,10 +293,7 @@ This uses reaction. } // Create file tracker - tracker, err := NewFileTracker() - if err != nil { - t.Fatalf("Failed to create file tracker: %v", err) - } + tracker := NewFileTracker() // Compile the workflow with tracking if err := compileWorkflowWithTracking(workflowFileWithReaction, false, false, "", tracker); err != nil { @@ -343,10 +332,7 @@ This does NOT use ai-reaction. } // Create new file tracker for second test - tracker2, err := NewFileTracker() - if err != nil { - t.Fatalf("Failed to create file tracker: %v", err) - } + tracker2 := NewFileTracker() // Remove the existing reaction action to test it's not created again // (Note: Since reaction is now inline, this removal step is no longer needed) @@ -359,3 +345,35 @@ This does NOT use ai-reaction. // Note: Since reaction feature now uses inline GitHub Scripts instead of separate action files, // we don't expect any reaction action files to be created or tracked } + +func TestFileTracker_StageAllFiles_NonGitRepo(t *testing.T) { + tempDir, err := os.MkdirTemp("", "file-tracker-non-git") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tempDir) + + oldWd, _ := os.Getwd() + defer func() { + _ = os.Chdir(oldWd) + }() + if err := os.Chdir(tempDir); err != nil { + t.Fatalf("Failed to change to temp directory: %v", err) + } + + testFile := filepath.Join(tempDir, "test.md") + if err := os.WriteFile(testFile, []byte("content"), 0644); err != nil { + t.Fatalf("Failed to write test file: %v", err) + } + + tracker := NewFileTracker() + tracker.TrackCreated(testFile) + + err = tracker.StageAllFiles(false) + if err == nil { + t.Fatal("Expected staging in non-git directory to fail") + } + if !strings.Contains(err.Error(), "file tracker requires being in a git repository") { + t.Fatalf("Expected non-git error message, got: %v", err) + } +} diff --git a/pkg/cli/packages_test.go b/pkg/cli/packages_test.go index b18e5ade120..f65607ae0b5 100644 --- a/pkg/cli/packages_test.go +++ b/pkg/cli/packages_test.go @@ -399,10 +399,7 @@ func TestCopyIncludeDependenciesFromPackageWithForce_FileTracker(t *testing.T) { t.Fatalf("Failed to write source file: %v", err) } - tracker, err := NewFileTracker() - if err != nil { - t.Fatalf("Failed to create file tracker: %v", err) - } + tracker := NewFileTracker() // Test 1: New file should be tracked as created deps := []IncludeDependency{ @@ -413,7 +410,7 @@ func TestCopyIncludeDependenciesFromPackageWithForce_FileTracker(t *testing.T) { }, } - err = copyIncludeDependenciesFromPackageWithForce(deps, targetDir, false, false, tracker) + err := copyIncludeDependenciesFromPackageWithForce(deps, targetDir, false, false, tracker) if err != nil { t.Errorf("Unexpected error: %v", err) } @@ -425,10 +422,7 @@ func TestCopyIncludeDependenciesFromPackageWithForce_FileTracker(t *testing.T) { } // Test 2: Modified file should be tracked as modified - tracker2, err := NewFileTracker() - if err != nil { - t.Fatalf("Failed to create file tracker: %v", err) - } + tracker2 := NewFileTracker() // Update source content if err := os.WriteFile(sourceFile, []byte("New Content"), 0644); err != nil { t.Fatalf("Failed to update source file: %v", err)