diff --git a/cmd/finish.go b/cmd/finish.go index c2822ab..14d67a3 100644 --- a/cmd/finish.go +++ b/cmd/finish.go @@ -747,6 +747,14 @@ func handleDeleteBranchStep(cfg *config.Config, state *mergestate.MergeState, re return &errors.GitError{Operation: fmt.Sprintf("checkout parent branch '%s'", state.ParentBranch), Err: err} } + // Clear the merge state before branch deletion. By this point all merges, + // tags, and child updates are complete — the state is only needed for conflict + // recovery which is no longer possible. Clearing early ensures a failed branch + // deletion (e.g. remote permission error) doesn't leave stale merge state. + if err := mergestate.ClearMergeState(); err != nil { + return &errors.GitError{Operation: "clear merge state", Err: err} + } + // Apply keep logic: if keep is set, it overrides individual settings keepRemote := resolvedOptions.KeepRemote keepLocal := resolvedOptions.KeepLocal @@ -770,11 +778,6 @@ func handleDeleteBranchStep(cfg *config.Config, state *mergestate.MergeState, re } } - // Clear the merge state - if err := mergestate.ClearMergeState(); err != nil { - return &errors.GitError{Operation: "clear merge state", Err: err} - } - fmt.Printf("Successfully finished branch '%s' and updated %d child base branches\n", state.FullBranchName, len(state.UpdatedBranches)) // Run post-hook after successful completion diff --git a/test/cmd/finish_retention_test.go b/test/cmd/finish_retention_test.go index 6edeb74..fb86fc2 100644 --- a/test/cmd/finish_retention_test.go +++ b/test/cmd/finish_retention_test.go @@ -419,3 +419,84 @@ func TestFinishDeleteBranchUsesConfiguredRemote(t *testing.T) { t.Error("Expected remote branch to be deleted from upstream") } } + +// TestFinishClearsMergeStateWhenBranchDeletionFails tests that merge state is cleared even when +// remote branch deletion fails during finish. +// Steps: +// 1. Sets up a test repository with a remote and initializes git-flow +// 2. Creates a feature branch with changes and pushes to remote +// 3. Configures the remote repository to reject branch deletions so deletion will fail +// 4. Finishes the feature branch (merge succeeds, remote deletion fails) +// 5. Verifies the merge state file does not exist (cleared despite deletion error) +// 6. Verifies the merge completed successfully (changes are on develop) +func TestFinishClearsMergeStateWhenBranchDeletionFails(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) + } + + // Add a remote repository + remoteDir, err := testutil.AddRemote(t, dir, "origin", true) + if err != nil { + t.Fatalf("Failed to add remote: %v", err) + } + defer testutil.CleanupTestRepo(t, remoteDir) + + // Create a feature branch with changes + output, err = testutil.RunGitFlow(t, dir, "feature", "start", "delete-fail-test") + if err != nil { + t.Fatalf("Failed to start feature branch: %v\nOutput: %s", err, output) + } + + testutil.WriteFile(t, dir, "feature-file.txt", "feature content") + _, err = testutil.RunGit(t, dir, "add", "feature-file.txt") + if err != nil { + t.Fatalf("Failed to add file: %v", err) + } + _, err = testutil.RunGit(t, dir, "commit", "-m", "Add feature file") + if err != nil { + t.Fatalf("Failed to commit: %v", err) + } + + // Push feature branch to remote + _, err = testutil.RunGit(t, dir, "push", "origin", "feature/delete-fail-test") + if err != nil { + t.Fatalf("Failed to push feature branch: %v", err) + } + + // Configure the remote bare repo to reject branch deletions + _, err = testutil.RunGit(t, remoteDir, "config", "receive.denyDeletes", "true") + if err != nil { + t.Fatalf("Failed to configure receive.denyDeletes: %v", err) + } + + // Finish the feature branch — merge should succeed but remote deletion should fail + output, err = testutil.RunGitFlow(t, dir, "feature", "finish", "delete-fail-test") + _ = output // finish may or may not return an error depending on how deletion failure is handled + + // Verify remote branch still exists (confirms deletion was actually rejected) + remoteRefs, err := testutil.RunGit(t, dir, "ls-remote", "--heads", "origin", "feature/delete-fail-test") + if err != nil { + t.Fatalf("Failed to list remote refs: %v", err) + } + if remoteRefs == "" { + t.Fatal("Expected remote branch feature/delete-fail-test to still exist, but it was deleted") + } + + // KEY ASSERTION: merge state must be cleared even though deletion failed + if testutil.GitFlowMergeStateExists(t, dir) { + t.Error("Expected merge state to be cleared after finish, but merge.json still exists") + } + + // Verify the merge itself completed — feature changes should be on develop + _, err = testutil.RunGit(t, dir, "checkout", "develop") + if err != nil { + t.Fatalf("Failed to checkout develop: %v", err) + } + if !testutil.FileExists(t, dir, "feature-file.txt") { + t.Error("Expected feature-file.txt to exist on develop after merge") + } +} diff --git a/test/testutil/testutil.go b/test/testutil/testutil.go index 1dd73fb..ecb4c10 100644 --- a/test/testutil/testutil.go +++ b/test/testutil/testutil.go @@ -86,3 +86,22 @@ 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) + } + stateFile := filepath.Join(gitDir, "gitflow", "state", "merge.json") + _, err = os.Stat(stateFile) + if err == nil { + return true + } + if os.IsNotExist(err) { + return false + } + t.Fatalf("Failed to stat git-flow merge state file %s: %v", stateFile, err) + return false +}