Align pkg/cli constructor semantics by making NewFileTracker lazy and non-erroring

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)
 }
 

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() {

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

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)
+	}
+}

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)