From 6abc9a15167f5634172f3b9689204aa1006ee424 Mon Sep 17 00:00:00 2001 From: Alexander Rinass Date: Wed, 8 Apr 2026 13:33:51 +0200 Subject: [PATCH 1/3] feat(finish): Detect and auto-clear stale merge state Adds validation to IsMergeInProgress() so that stale merge state files are automatically detected and cleared instead of blocking operations with confusing errors. The validation checks three things: critical fields must be non-empty, conflict steps (merge, update_children) must have an actual git merge/rebase/squash-merge in progress, and non-conflict steps (create_tag, delete_branch) must reference a branch that still exists. Three new git state detection functions are added to internal/git: IsGitMergeInProgress (checks MERGE_HEAD), IsGitRebaseInProgress (checks rebase-merge/ and rebase-apply/), and IsGitSquashMergeInProgress (checks SQUASH_MSG without rebase-merge/, since SQUASH_MSG also appears during interactive rebase squash steps). When stale state is detected, it is cleared and a note is printed to stderr so the user knows what happened. Resolves #101 --- internal/git/repo.go | 46 +++++++ internal/mergestate/mergestate.go | 35 +++++- test/cmd/finish_state_test.go | 190 ++++++++++++++++++++++++++++ test/internal/git/repo_test.go | 196 +++++++++++++++++++++++++++++ test/internal/git/worktree_test.go | 21 ++-- test/testutil/testutil.go | 33 +++++ 6 files changed, 511 insertions(+), 10 deletions(-) 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..9d9343a 100644 --- a/internal/mergestate/mergestate.go +++ b/internal/mergestate/mergestate.go @@ -125,8 +125,39 @@ 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 || state == nil { + return false + } + if !isStateValid(state) { + fmt.Fprintf(os.Stderr, "Note: Cleared stale merge state from a previous operation\n") + ClearMergeState() + return false + } + return true } diff --git a/test/cmd/finish_state_test.go b/test/cmd/finish_state_test.go index 7b4e065..5ca8f5d 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,192 @@ 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") + testutil.RunGit(t, dir, "add", "new.txt") + testutil.RunGit(t, dir, "commit", "-m", "New feature commit") + + 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") + testutil.RunGit(t, dir, "add", "fresh.txt") + testutil.RunGit(t, dir, "commit", "-m", "Fresh feature commit") + + 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") + testutil.RunGit(t, dir, "add", "another.txt") + testutil.RunGit(t, dir, "commit", "-m", "Another feature commit") + + 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") + testutil.RunGit(t, dir, "add", "conflict.txt") + testutil.RunGit(t, dir, "commit", "-m", "Feature commit") + + // Add conflicting content on develop + testutil.RunGit(t, dir, "checkout", "develop") + testutil.WriteFile(t, dir, "conflict.txt", "develop content") + testutil.RunGit(t, dir, "add", "conflict.txt") + testutil.RunGit(t, dir, "commit", "-m", "Develop commit") + + // Switch back and finish — will produce conflict + testutil.RunGit(t, dir, "checkout", "feature/conflict-test") + 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..2c4ea96 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 { + return false + } + _, 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) + } +} From f8bb672ba9f61c377bee16ebc6eb5dc38aaeeb11 Mon Sep 17 00:00:00 2001 From: Alexander Rinass Date: Wed, 8 Apr 2026 14:43:06 +0200 Subject: [PATCH 2/3] fix(mergestate): Address review feedback on stale state handling Clear corrupted merge state files on LoadMergeState errors instead of silently leaving them on disk. Only print the stale-state note when ClearMergeState succeeds. Add error checks to git setup commands in stale-state integration tests. Fail GitFlowMergeStateExists on git dir errors instead of silently returning false. Co-Authored-By: Claude Opus 4.6 --- internal/mergestate/mergestate.go | 14 ++++++-- test/cmd/finish_state_test.go | 60 ++++++++++++++++++++++++------- test/testutil/testutil.go | 2 +- 3 files changed, 60 insertions(+), 16 deletions(-) diff --git a/internal/mergestate/mergestate.go b/internal/mergestate/mergestate.go index 9d9343a..083bf3f 100644 --- a/internal/mergestate/mergestate.go +++ b/internal/mergestate/mergestate.go @@ -151,12 +151,20 @@ func isStateValid(state *MergeState) bool { // it is automatically cleared and false is returned. func IsMergeInProgress() bool { state, err := LoadMergeState() - if err != nil || state == nil { + if err != nil { + // Corrupted or unreadable state file — clear it + if clearErr := ClearMergeState(); clearErr == nil { + fmt.Fprintf(os.Stderr, "Note: Cleared stale merge state from a previous operation\n") + } + return false + } + if state == nil { return false } if !isStateValid(state) { - fmt.Fprintf(os.Stderr, "Note: Cleared stale merge state from a previous operation\n") - ClearMergeState() + if err := ClearMergeState(); err == nil { + 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 5ca8f5d..ffa812e 100644 --- a/test/cmd/finish_state_test.go +++ b/test/cmd/finish_state_test.go @@ -321,8 +321,14 @@ func TestFinishDetectsStaleStateEmptyFields(t *testing.T) { t.Fatalf("Failed to start feature: %v\nOutput: %s", err, output) } testutil.WriteFile(t, dir, "new.txt", "content") - testutil.RunGit(t, dir, "add", "new.txt") - testutil.RunGit(t, dir, "commit", "-m", "New feature commit") + _, 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 { @@ -368,8 +374,14 @@ func TestFinishDetectsStaleStateMergeStepNoConflict(t *testing.T) { t.Fatalf("Failed to start feature: %v\nOutput: %s", err, output) } testutil.WriteFile(t, dir, "fresh.txt", "content") - testutil.RunGit(t, dir, "add", "fresh.txt") - testutil.RunGit(t, dir, "commit", "-m", "Fresh feature commit") + _, 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 { @@ -414,8 +426,14 @@ func TestFinishDetectsStaleStateDeleteStepBranchGone(t *testing.T) { t.Fatalf("Failed to start feature: %v\nOutput: %s", err, output) } testutil.WriteFile(t, dir, "another.txt", "content") - testutil.RunGit(t, dir, "add", "another.txt") - testutil.RunGit(t, dir, "commit", "-m", "Another feature commit") + _, 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 { @@ -449,17 +467,35 @@ func TestFinishValidStateMergeStepWithConflict(t *testing.T) { t.Fatalf("Failed to start feature: %v\nOutput: %s", err, output) } testutil.WriteFile(t, dir, "conflict.txt", "feature content") - testutil.RunGit(t, dir, "add", "conflict.txt") - testutil.RunGit(t, dir, "commit", "-m", "Feature commit") + _, 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 - testutil.RunGit(t, dir, "checkout", "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") - testutil.RunGit(t, dir, "add", "conflict.txt") - testutil.RunGit(t, dir, "commit", "-m", "Develop commit") + _, 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 - testutil.RunGit(t, dir, "checkout", "feature/conflict-test") + _, 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 diff --git a/test/testutil/testutil.go b/test/testutil/testutil.go index 2c4ea96..ff350fa 100644 --- a/test/testutil/testutil.go +++ b/test/testutil/testutil.go @@ -92,7 +92,7 @@ func GitFlowMergeStateExists(t *testing.T, dir string) bool { t.Helper() gitDir, err := getGitDirForRepo(dir) if err != nil { - return false + 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 From b338505606755c77d88d0a503fdfd3f2418697b0 Mon Sep 17 00:00:00 2001 From: Alexander Rinass Date: Wed, 8 Apr 2026 15:25:55 +0200 Subject: [PATCH 3/3] fix(mergestate): Warn when stale state clear fails Emit a warning to stderr when ClearMergeState fails instead of silently ignoring the error. This helps users understand why stale merge state keeps reappearing (e.g., permission issues). Co-Authored-By: Claude Opus 4.6 --- internal/mergestate/mergestate.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/mergestate/mergestate.go b/internal/mergestate/mergestate.go index 083bf3f..3fd785d 100644 --- a/internal/mergestate/mergestate.go +++ b/internal/mergestate/mergestate.go @@ -153,7 +153,9 @@ func IsMergeInProgress() bool { state, err := LoadMergeState() if err != nil { // Corrupted or unreadable state file — clear it - if clearErr := ClearMergeState(); clearErr == nil { + 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 @@ -162,7 +164,9 @@ func IsMergeInProgress() bool { return false } if !isStateValid(state) { - if err := ClearMergeState(); err == nil { + 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