diff --git a/internal/git/repo.go b/internal/git/repo.go index 95347a6..496c098 100644 --- a/internal/git/repo.go +++ b/internal/git/repo.go @@ -2,7 +2,9 @@ package git import ( "fmt" + "os" "os/exec" + "path/filepath" "strconv" "strings" ) @@ -275,6 +277,50 @@ func HasConflicts() bool { return len(output) > 0 } +// IsGitMergeInProgress checks if git is in a merge state by looking for MERGE_HEAD +func IsGitMergeInProgress() bool { + gitDir, err := GetGitDir() + if err != nil { + return false + } + _, err = os.Stat(filepath.Join(gitDir, "MERGE_HEAD")) + return err == nil +} + +// IsGitRebaseInProgress checks if git is in a rebase state by looking for +// rebase-merge/ (merge backend, including interactive) or rebase-apply/ (legacy apply backend) +func IsGitRebaseInProgress() bool { + gitDir, err := GetGitDir() + if err != nil { + return false + } + if _, err := os.Stat(filepath.Join(gitDir, "rebase-merge")); err == nil { + return true + } + if _, err := os.Stat(filepath.Join(gitDir, "rebase-apply")); err == nil { + return true + } + return false +} + +// IsGitSquashMergeInProgress checks if git is in a squash merge state. +// Squash merges create SQUASH_MSG but not MERGE_HEAD. However, SQUASH_MSG also +// appears during interactive rebase squash steps, so we exclude that case. +func IsGitSquashMergeInProgress() bool { + gitDir, err := GetGitDir() + if err != nil { + return false + } + if _, err := os.Stat(filepath.Join(gitDir, "SQUASH_MSG")); err != nil { + return false + } + // Exclude interactive rebase squash steps + if _, err := os.Stat(filepath.Join(gitDir, "rebase-merge")); err == nil { + return false + } + return true +} + // MergeAbort aborts the current merge func MergeAbort() error { cmd := exec.Command("git", "merge", "--abort") diff --git a/internal/mergestate/mergestate.go b/internal/mergestate/mergestate.go index 102efee..3fd785d 100644 --- a/internal/mergestate/mergestate.go +++ b/internal/mergestate/mergestate.go @@ -125,8 +125,51 @@ func ClearMergeState() error { return nil } -// IsMergeInProgress checks if there's a merge in progress +// isStateValid checks whether a loaded merge state is still meaningful by +// validating it against the actual git repository state. This detects stale +// state files left by manual intervention, crashes, or interrupted operations. +func isStateValid(state *MergeState) bool { + // Critical fields must be non-empty + if state.BranchType == "" || state.FullBranchName == "" || state.CurrentStep == "" { + return false + } + + switch state.CurrentStep { + case "merge", "update_children": + // Git must actually be in a merge, rebase, or squash merge state + return git.IsGitMergeInProgress() || git.IsGitRebaseInProgress() || git.IsGitSquashMergeInProgress() + case "create_tag", "delete_branch": + // The topic branch must still exist + return git.BranchExists(state.FullBranchName) == nil + default: + return false + } +} + +// IsMergeInProgress checks if there's a valid merge in progress. If a state +// file exists but is stale (e.g., manual resolution, crash, or missing branch), +// it is automatically cleared and false is returned. func IsMergeInProgress() bool { state, err := LoadMergeState() - return err == nil && state != nil + if err != nil { + // Corrupted or unreadable state file — clear it + if clearErr := ClearMergeState(); clearErr != nil { + fmt.Fprintf(os.Stderr, "Warning: Failed to clear stale merge state: %v\n", clearErr) + } else { + fmt.Fprintf(os.Stderr, "Note: Cleared stale merge state from a previous operation\n") + } + return false + } + if state == nil { + return false + } + if !isStateValid(state) { + if err := ClearMergeState(); err != nil { + fmt.Fprintf(os.Stderr, "Warning: Failed to clear stale merge state: %v\n", err) + } else { + fmt.Fprintf(os.Stderr, "Note: Cleared stale merge state from a previous operation\n") + } + return false + } + return true } diff --git a/test/cmd/finish_state_test.go b/test/cmd/finish_state_test.go index 7b4e065..ffa812e 100644 --- a/test/cmd/finish_state_test.go +++ b/test/cmd/finish_state_test.go @@ -4,6 +4,7 @@ import ( "strings" "testing" + "github.com/gittower/git-flow-next/internal/mergestate" "github.com/gittower/git-flow-next/test/testutil" ) @@ -290,3 +291,228 @@ func TestFinishStateBackwardCompatibility(t *testing.T) { t.Error("Feature changes not found in develop branch") } } + +// TestFinishDetectsStaleStateEmptyFields tests that stale state with empty critical fields is auto-cleared. +// Steps: +// 1. Sets up a test repository and initializes git-flow +// 2. Writes a merge state file with empty BranchType +// 3. Creates and finishes a feature branch normally +// 4. Verifies the stale state was cleared and finish succeeded +func TestFinishDetectsStaleStateEmptyFields(t *testing.T) { + dir := testutil.SetupTestRepo(t) + defer testutil.CleanupTestRepo(t, dir) + + output, err := testutil.RunGitFlow(t, dir, "init", "--defaults") + if err != nil { + t.Fatalf("Failed to initialize git-flow: %v\nOutput: %s", err, output) + } + + // Write stale state with empty BranchType + testutil.WriteMergeState(t, dir, &mergestate.MergeState{ + Action: "finish", + BranchType: "", // empty — should be detected as stale + FullBranchName: "feature/old-branch", + CurrentStep: "merge", + }) + + // Create and finish a new feature — should succeed because stale state is cleared + output, err = testutil.RunGitFlow(t, dir, "feature", "start", "new-feature") + if err != nil { + t.Fatalf("Failed to start feature: %v\nOutput: %s", err, output) + } + testutil.WriteFile(t, dir, "new.txt", "content") + _, err = testutil.RunGit(t, dir, "add", "new.txt") + if err != nil { + t.Fatalf("Failed to add file: %v", err) + } + _, err = testutil.RunGit(t, dir, "commit", "-m", "New feature commit") + if err != nil { + t.Fatalf("Failed to commit: %v", err) + } + + output, err = testutil.RunGitFlow(t, dir, "feature", "finish", "new-feature") + if err != nil { + t.Fatalf("Expected finish to succeed after clearing stale state, got error: %v\nOutput: %s", err, output) + } + + // Verify stale state was cleared + if testutil.GitFlowMergeStateExists(t, dir) { + t.Error("Expected merge state to be cleared") + } +} + +// TestFinishDetectsStaleStateMergeStepNoConflict tests that state at merge step is cleared when +// git is not actually in a merge or rebase state. +// Steps: +// 1. Sets up a test repository and initializes git-flow +// 2. Writes a merge state file at the merge step +// 3. Runs feature finish which checks for merge in progress +// 4. Verifies stale state is cleared and a new finish can proceed +func TestFinishDetectsStaleStateMergeStepNoConflict(t *testing.T) { + dir := testutil.SetupTestRepo(t) + defer testutil.CleanupTestRepo(t, dir) + + output, err := testutil.RunGitFlow(t, dir, "init", "--defaults") + if err != nil { + t.Fatalf("Failed to initialize git-flow: %v\nOutput: %s", err, output) + } + + // Write stale state claiming merge step but git is not in merge state + testutil.WriteMergeState(t, dir, &mergestate.MergeState{ + Action: "finish", + BranchType: "feature", + BranchName: "old-branch", + FullBranchName: "feature/old-branch", + CurrentStep: "merge", + ParentBranch: "develop", + MergeStrategy: "merge", + }) + + // Create and finish a new feature — stale state should be auto-cleared + output, err = testutil.RunGitFlow(t, dir, "feature", "start", "fresh-feature") + if err != nil { + t.Fatalf("Failed to start feature: %v\nOutput: %s", err, output) + } + testutil.WriteFile(t, dir, "fresh.txt", "content") + _, err = testutil.RunGit(t, dir, "add", "fresh.txt") + if err != nil { + t.Fatalf("Failed to add file: %v", err) + } + _, err = testutil.RunGit(t, dir, "commit", "-m", "Fresh feature commit") + if err != nil { + t.Fatalf("Failed to commit: %v", err) + } + + output, err = testutil.RunGitFlow(t, dir, "feature", "finish", "fresh-feature") + if err != nil { + t.Fatalf("Expected finish to succeed after clearing stale state, got error: %v\nOutput: %s", err, output) + } + + if strings.Contains(output, "merge in progress") { + t.Error("Expected stale state to be cleared, but got merge in progress error") + } +} + +// TestFinishDetectsStaleStateDeleteStepBranchGone tests that state at delete_branch step is +// cleared when the topic branch no longer exists. +// Steps: +// 1. Sets up a test repository and initializes git-flow +// 2. Writes a merge state file at delete_branch step referencing a non-existent branch +// 3. Runs feature finish to verify stale state is cleared +// 4. Verifies a new finish can proceed normally +func TestFinishDetectsStaleStateDeleteStepBranchGone(t *testing.T) { + dir := testutil.SetupTestRepo(t) + defer testutil.CleanupTestRepo(t, dir) + + output, err := testutil.RunGitFlow(t, dir, "init", "--defaults") + if err != nil { + t.Fatalf("Failed to initialize git-flow: %v\nOutput: %s", err, output) + } + + // Write stale state at delete_branch step for a branch that doesn't exist + testutil.WriteMergeState(t, dir, &mergestate.MergeState{ + Action: "finish", + BranchType: "feature", + BranchName: "deleted-branch", + FullBranchName: "feature/deleted-branch", + CurrentStep: "delete_branch", + ParentBranch: "develop", + MergeStrategy: "merge", + }) + + // Create and finish a new feature — stale state should be auto-cleared + output, err = testutil.RunGitFlow(t, dir, "feature", "start", "another-feature") + if err != nil { + t.Fatalf("Failed to start feature: %v\nOutput: %s", err, output) + } + testutil.WriteFile(t, dir, "another.txt", "content") + _, err = testutil.RunGit(t, dir, "add", "another.txt") + if err != nil { + t.Fatalf("Failed to add file: %v", err) + } + _, err = testutil.RunGit(t, dir, "commit", "-m", "Another feature commit") + if err != nil { + t.Fatalf("Failed to commit: %v", err) + } + + output, err = testutil.RunGitFlow(t, dir, "feature", "finish", "another-feature") + if err != nil { + t.Fatalf("Expected finish to succeed after clearing stale state, got error: %v\nOutput: %s", err, output) + } + + if testutil.GitFlowMergeStateExists(t, dir) { + t.Error("Expected merge state to be cleared after finish") + } +} + +// TestFinishValidStateMergeStepWithConflict tests that legitimate merge state is NOT cleared +// when git is actually in a merge conflict state. +// Steps: +// 1. Sets up a test repository and initializes git-flow +// 2. Creates a feature branch with conflicting changes +// 3. Attempts to finish (produces merge conflict, creating valid state) +// 4. Verifies the merge state is preserved (not cleared as stale) +func TestFinishValidStateMergeStepWithConflict(t *testing.T) { + dir := testutil.SetupTestRepo(t) + defer testutil.CleanupTestRepo(t, dir) + + output, err := testutil.RunGitFlow(t, dir, "init", "--defaults") + if err != nil { + t.Fatalf("Failed to initialize git-flow: %v\nOutput: %s", err, output) + } + + // Create feature with content + output, err = testutil.RunGitFlow(t, dir, "feature", "start", "conflict-test") + if err != nil { + t.Fatalf("Failed to start feature: %v\nOutput: %s", err, output) + } + testutil.WriteFile(t, dir, "conflict.txt", "feature content") + _, err = testutil.RunGit(t, dir, "add", "conflict.txt") + if err != nil { + t.Fatalf("Failed to add file: %v", err) + } + _, err = testutil.RunGit(t, dir, "commit", "-m", "Feature commit") + if err != nil { + t.Fatalf("Failed to commit: %v", err) + } + + // Add conflicting content on develop + _, err = testutil.RunGit(t, dir, "checkout", "develop") + if err != nil { + t.Fatalf("Failed to checkout develop: %v", err) + } + testutil.WriteFile(t, dir, "conflict.txt", "develop content") + _, err = testutil.RunGit(t, dir, "add", "conflict.txt") + if err != nil { + t.Fatalf("Failed to add file: %v", err) + } + _, err = testutil.RunGit(t, dir, "commit", "-m", "Develop commit") + if err != nil { + t.Fatalf("Failed to commit: %v", err) + } + + // Switch back and finish — will produce conflict + _, err = testutil.RunGit(t, dir, "checkout", "feature/conflict-test") + if err != nil { + t.Fatalf("Failed to checkout feature branch: %v", err) + } + output, err = testutil.RunGitFlow(t, dir, "feature", "finish", "conflict-test") + + // Finish should fail with conflict + if err == nil { + t.Fatal("Expected finish to fail with merge conflict") + } + + // The merge state should be preserved — this is a legitimate conflict + if !testutil.GitFlowMergeStateExists(t, dir) { + t.Error("Expected merge state to be preserved during legitimate conflict") + } + + state, stateErr := testutil.LoadMergeState(t, dir) + if stateErr != nil { + t.Fatalf("Failed to load merge state: %v", stateErr) + } + if state.CurrentStep != "merge" { + t.Errorf("Expected state step 'merge', got '%s'", state.CurrentStep) + } +} diff --git a/test/internal/git/repo_test.go b/test/internal/git/repo_test.go index 0bbb1c2..82cfb49 100644 --- a/test/internal/git/repo_test.go +++ b/test/internal/git/repo_test.go @@ -667,3 +667,199 @@ func TestFetchBranchNonExistent(t *testing.T) { } }) } + +// TestIsGitMergeInProgressTrue tests detection of an active git merge conflict. +// Steps: +// 1. Sets up a test repository with two branches containing conflicting changes +// 2. Starts a merge that produces a conflict +// 3. Verifies IsGitMergeInProgress returns true +func TestIsGitMergeInProgressTrue(t *testing.T) { + dir := testutil.SetupTestRepo(t) + defer testutil.CleanupTestRepo(t, dir) + + // Create a branch with content + _, err := testutil.RunGit(t, dir, "checkout", "-b", "feature") + if err != nil { + t.Fatalf("Failed to create branch: %v", err) + } + testutil.WriteFile(t, dir, "conflict.txt", "feature content") + _, err = testutil.RunGit(t, dir, "add", "conflict.txt") + if err != nil { + t.Fatalf("Failed to add file: %v", err) + } + _, err = testutil.RunGit(t, dir, "commit", "-m", "Feature commit") + if err != nil { + t.Fatalf("Failed to commit: %v", err) + } + + // Add conflicting content on main + _, err = testutil.RunGit(t, dir, "checkout", "main") + if err != nil { + t.Fatalf("Failed to checkout main: %v", err) + } + testutil.WriteFile(t, dir, "conflict.txt", "main content") + _, err = testutil.RunGit(t, dir, "add", "conflict.txt") + if err != nil { + t.Fatalf("Failed to add file: %v", err) + } + _, err = testutil.RunGit(t, dir, "commit", "-m", "Main commit") + if err != nil { + t.Fatalf("Failed to commit: %v", err) + } + + // Start merge that will conflict + _, _ = testutil.RunGit(t, dir, "merge", "feature") + + withGitRepo(t, dir, func() { + if !git.IsGitMergeInProgress() { + t.Error("Expected IsGitMergeInProgress to return true during merge conflict") + } + }) +} + +// TestIsGitMergeInProgressFalse tests that clean repos are not detected as merging. +// Steps: +// 1. Sets up a clean test repository +// 2. Verifies IsGitMergeInProgress returns false +func TestIsGitMergeInProgressFalse(t *testing.T) { + dir := testutil.SetupTestRepo(t) + defer testutil.CleanupTestRepo(t, dir) + + withGitRepo(t, dir, func() { + if git.IsGitMergeInProgress() { + t.Error("Expected IsGitMergeInProgress to return false on clean repo") + } + }) +} + +// TestIsGitRebaseInProgressTrue tests detection of an active git rebase conflict. +// Steps: +// 1. Sets up a test repository with two branches containing conflicting changes +// 2. Starts a rebase that produces a conflict +// 3. Verifies IsGitRebaseInProgress returns true +func TestIsGitRebaseInProgressTrue(t *testing.T) { + dir := testutil.SetupTestRepo(t) + defer testutil.CleanupTestRepo(t, dir) + + // Create a branch with content + _, err := testutil.RunGit(t, dir, "checkout", "-b", "feature") + if err != nil { + t.Fatalf("Failed to create branch: %v", err) + } + testutil.WriteFile(t, dir, "conflict.txt", "feature content") + _, err = testutil.RunGit(t, dir, "add", "conflict.txt") + if err != nil { + t.Fatalf("Failed to add file: %v", err) + } + _, err = testutil.RunGit(t, dir, "commit", "-m", "Feature commit") + if err != nil { + t.Fatalf("Failed to commit: %v", err) + } + + // Add conflicting content on main + _, err = testutil.RunGit(t, dir, "checkout", "main") + if err != nil { + t.Fatalf("Failed to checkout main: %v", err) + } + testutil.WriteFile(t, dir, "conflict.txt", "main content") + _, err = testutil.RunGit(t, dir, "add", "conflict.txt") + if err != nil { + t.Fatalf("Failed to add file: %v", err) + } + _, err = testutil.RunGit(t, dir, "commit", "-m", "Main commit") + if err != nil { + t.Fatalf("Failed to commit: %v", err) + } + + // Switch to feature and rebase onto main (will conflict) + _, err = testutil.RunGit(t, dir, "checkout", "feature") + if err != nil { + t.Fatalf("Failed to checkout feature: %v", err) + } + _, _ = testutil.RunGit(t, dir, "rebase", "main") + + withGitRepo(t, dir, func() { + if !git.IsGitRebaseInProgress() { + t.Error("Expected IsGitRebaseInProgress to return true during rebase conflict") + } + }) +} + +// TestIsGitRebaseInProgressFalse tests that clean repos are not detected as rebasing. +// Steps: +// 1. Sets up a clean test repository +// 2. Verifies IsGitRebaseInProgress returns false +func TestIsGitRebaseInProgressFalse(t *testing.T) { + dir := testutil.SetupTestRepo(t) + defer testutil.CleanupTestRepo(t, dir) + + withGitRepo(t, dir, func() { + if git.IsGitRebaseInProgress() { + t.Error("Expected IsGitRebaseInProgress to return false on clean repo") + } + }) +} + +// TestIsGitSquashMergeInProgressTrue tests detection of an active squash merge conflict. +// Steps: +// 1. Sets up a test repository with two branches containing conflicting changes +// 2. Starts a squash merge that produces a conflict +// 3. Verifies IsGitSquashMergeInProgress returns true +func TestIsGitSquashMergeInProgressTrue(t *testing.T) { + dir := testutil.SetupTestRepo(t) + defer testutil.CleanupTestRepo(t, dir) + + // Create a branch with content + _, err := testutil.RunGit(t, dir, "checkout", "-b", "feature") + if err != nil { + t.Fatalf("Failed to create branch: %v", err) + } + testutil.WriteFile(t, dir, "conflict.txt", "feature content") + _, err = testutil.RunGit(t, dir, "add", "conflict.txt") + if err != nil { + t.Fatalf("Failed to add file: %v", err) + } + _, err = testutil.RunGit(t, dir, "commit", "-m", "Feature commit") + if err != nil { + t.Fatalf("Failed to commit: %v", err) + } + + // Add conflicting content on main + _, err = testutil.RunGit(t, dir, "checkout", "main") + if err != nil { + t.Fatalf("Failed to checkout main: %v", err) + } + testutil.WriteFile(t, dir, "conflict.txt", "main content") + _, err = testutil.RunGit(t, dir, "add", "conflict.txt") + if err != nil { + t.Fatalf("Failed to add file: %v", err) + } + _, err = testutil.RunGit(t, dir, "commit", "-m", "Main commit") + if err != nil { + t.Fatalf("Failed to commit: %v", err) + } + + // Start squash merge that will conflict + _, _ = testutil.RunGit(t, dir, "merge", "--squash", "feature") + + withGitRepo(t, dir, func() { + if !git.IsGitSquashMergeInProgress() { + t.Error("Expected IsGitSquashMergeInProgress to return true during squash merge conflict") + } + }) +} + +// TestIsGitSquashMergeInProgressFalse tests that clean repos are not detected as squash merging. +// Steps: +// 1. Sets up a clean test repository +// 2. Verifies IsGitSquashMergeInProgress returns false +func TestIsGitSquashMergeInProgressFalse(t *testing.T) { + dir := testutil.SetupTestRepo(t) + defer testutil.CleanupTestRepo(t, dir) + + withGitRepo(t, dir, func() { + if git.IsGitSquashMergeInProgress() { + t.Error("Expected IsGitSquashMergeInProgress to return false on clean repo") + } + }) +} diff --git a/test/internal/git/worktree_test.go b/test/internal/git/worktree_test.go index 7598fa7..3ca98db 100644 --- a/test/internal/git/worktree_test.go +++ b/test/internal/git/worktree_test.go @@ -93,15 +93,16 @@ func TestMergeStateSaveLoadInWorktree(t *testing.T) { // Change to worktree and test merge state operations withGitRepo(t, worktreePath, func() { - // Create test state + // Create test state using delete_branch step with an existing branch so that + // IsMergeInProgress validation passes (it checks branch existence for non-conflict steps) state := &mergestate.MergeState{ Action: "finish", BranchType: "feature", BranchName: "test-feature", - CurrentStep: "merge", - ParentBranch: "develop", + CurrentStep: "delete_branch", + ParentBranch: "main", MergeStrategy: "merge", - FullBranchName: "feature/test-feature", + FullBranchName: "worktree-branch", } // Test save @@ -184,12 +185,16 @@ func TestMergeStateNotSharedBetweenWorktrees(t *testing.T) { t.Fatalf("Failed to create worktree2: %v", err) } - // Save state in worktree1 + // Save state in worktree1 using delete_branch step with an existing branch + // so that IsMergeInProgress validation passes withGitRepo(t, worktree1, func() { state := &mergestate.MergeState{ - Action: "finish", - BranchType: "feature", - BranchName: "worktree1-feature", + Action: "finish", + BranchType: "feature", + BranchName: "worktree1-feature", + CurrentStep: "delete_branch", + FullBranchName: "worktree1-branch", + ParentBranch: "main", } err := mergestate.SaveMergeState(state) if err != nil { diff --git a/test/testutil/testutil.go b/test/testutil/testutil.go index 1dd73fb..ff350fa 100644 --- a/test/testutil/testutil.go +++ b/test/testutil/testutil.go @@ -86,3 +86,36 @@ func FileExists(t *testing.T, dir string, path string) bool { _, err := os.Stat(fullPath) return err == nil } + +// GitFlowMergeStateExists checks if the git-flow merge state file exists +func GitFlowMergeStateExists(t *testing.T, dir string) bool { + t.Helper() + gitDir, err := getGitDirForRepo(dir) + if err != nil { + t.Fatalf("Failed to determine git directory for repo %s: %v", dir, err) + } + _, err = os.Stat(filepath.Join(gitDir, "gitflow", "state", "merge.json")) + return err == nil +} + +// WriteMergeState writes a merge state file directly to the test repository. +// This is used for testing stale state detection without going through the +// normal git-flow operations. +func WriteMergeState(t *testing.T, dir string, state *mergestate.MergeState) { + t.Helper() + gitDir, err := getGitDirForRepo(dir) + if err != nil { + t.Fatalf("Failed to determine git directory: %v", err) + } + stateDir := filepath.Join(gitDir, "gitflow", "state") + if err := os.MkdirAll(stateDir, 0755); err != nil { + t.Fatalf("Failed to create state directory: %v", err) + } + data, err := json.Marshal(state) + if err != nil { + t.Fatalf("Failed to marshal merge state: %v", err) + } + if err := os.WriteFile(filepath.Join(stateDir, "merge.json"), data, 0644); err != nil { + t.Fatalf("Failed to write merge state file: %v", err) + } +}