Detect and auto-clear stale merge state by alexrinass · Pull Request #106 · gittower/git-flow-next

alexrinass · 2026-04-08T11:34:16Z

Add validation to IsMergeInProgress() so that stale merge state files are automatically detected, cleared, and reported instead of blocking operations with confusing errors like unknown branch type: (see #87).

IsMergeInProgress() previously only checked whether merge.json existed. It now validates the state against reality before returning true. If validation fails, the state file is cleared and a note is printed to stderr.

Validation rules:

Field validation — BranchType, FullBranchName, and CurrentStep must be non-empty
Conflict steps (merge, update_children) — git must actually be in a merge, rebase, or squash merge state
Non-conflict steps (create_tag, delete_branch) — the topic branch must still exist

Three new git state detection functions are added: 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).

Resolves #101

Remarks

Squash merge detection required special handling — git merge --squash creates SQUASH_MSG but not MERGE_HEAD, and SQUASH_MSG also appears during interactive rebase squash steps
Two existing worktree tests were updated to use valid state (non-conflict step with existing branch) so they pass the new validation
All validation happens inside IsMergeInProgress() so every caller benefits automatically
The mergestate package avoids a config dependency — it does not validate that BranchType exists in config

Review focus:

internal/git/repo.go — three new state detection functions
internal/mergestate/mergestate.go — isStateValid() and updated IsMergeInProgress()
test/cmd/finish_state_test.go — four new stale state integration tests
test/internal/git/repo_test.go — six new unit tests for state detection

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

Copilot

Pull request overview

Adds validation to internal/mergestate.IsMergeInProgress() so stale merge.json state is detected and auto-cleared, reducing confusing “unknown branch type” failures and unblocking normal operations across all callers.

Changes:

Validate merge state contents and reconcile it with actual Git state; auto-clear invalid state and emit a note to stderr.
Add Git repository state detection helpers for merge, rebase, and squash-merge in internal/git/repo.go.
Expand test utilities and add/adjust integration + unit tests around stale-state clearing and Git state detection.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
`internal/mergestate/mergestate.go`	Adds `isStateValid()` and updates `IsMergeInProgress()` to validate/clear stale merge state.
`internal/git/repo.go`	Adds `IsGitMergeInProgress`, `IsGitRebaseInProgress`, `IsGitSquashMergeInProgress` helpers.
`test/cmd/finish_state_test.go`	Adds integration tests covering stale-state auto-clear scenarios.
`test/internal/git/repo_test.go`	Adds unit tests for the new Git in-progress detection helpers.
`test/internal/git/worktree_test.go`	Updates worktree merge state tests to satisfy the new validation rules.
`test/testutil/testutil.go`	Adds helpers to write and assert existence of git-flow merge state in tests.

Copilot · 2026-04-08T11:47:13Z

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


IsMergeInProgress returns false on LoadMergeState errors but does not clear a corrupted/unreadable merge.json. This leaves stale state on disk (and can keep printing confusing behavior later). Consider treating non-nil errors from LoadMergeState as stale state: print the note and attempt ClearMergeState (handling any ClearMergeState error) before returning false.

Suggested change

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

if err != nil {

fmt.Fprintf(os.Stderr, "Note: Cleared stale merge state from a previous operation\n")

if clearErr := ClearMergeState(); clearErr != nil {

fmt.Fprintf(os.Stderr, "Warning: Failed to clear stale merge state after load error: %v\n", clearErr)

}

return false

}

if state == nil {

return false

}

if !isStateValid(state) {

fmt.Fprintf(os.Stderr, "Note: Cleared stale merge state from a previous operation\n")

if err := ClearMergeState(); err != nil {

fmt.Fprintf(os.Stderr, "Warning: Failed to clear stale merge state: %v\n", err)

}

Copilot · 2026-04-08T11:47:14Z

+		fmt.Fprintf(os.Stderr, "Note: Cleared stale merge state from a previous operation\n")
+		ClearMergeState()


ClearMergeState() returns an error but the result is currently ignored. If the remove fails (permissions, locked file, etc.), IsMergeInProgress will keep reporting and re-printing the stale-state note indefinitely. Handle and report the ClearMergeState error (and consider only printing the note once the clear succeeds).

Suggested change

fmt.Fprintf(os.Stderr, "Note: Cleared stale merge state from a previous operation\n")

ClearMergeState()

if err := ClearMergeState(); err != nil {

fmt.Fprintf(os.Stderr, "Warning: failed to clear stale merge state from a previous operation: %v\n", err)

} else {

fmt.Fprintf(os.Stderr, "Note: Cleared stale merge state from a previous operation\n")

}

Copilot · 2026-04-08T11:47:14Z

+	t.Helper()
+	gitDir, err := getGitDirForRepo(dir)
+	if err != nil {
+		return false


GitFlowMergeStateExists silently returns false when getGitDirForRepo fails. That can produce false negatives and make tests pass even if the repo is misconfigured. Prefer failing the test (t.Fatalf) on git dir resolution errors so assertions are reliable.

Suggested change

return false

t.Fatalf("Failed to determine git directory for repo %s: %v", dir, err)

Copilot · 2026-04-08T11:47:14Z

+	// Start merge that will conflict
+	_, _ = testutil.RunGit(t, dir, "merge", "feature")
+


This test intentionally triggers a merge conflict, but it ignores the error from git merge. If the merge unexpectedly succeeds (or fails for a different reason), the assertion on IsGitMergeInProgress becomes less meaningful. Capture the output/error and assert that the merge failed due to conflicts before checking IsGitMergeInProgress.

Copilot · 2026-04-08T11:47:15Z

+	// 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")
+


This test intentionally triggers a rebase conflict, but it ignores the error from git rebase. Assert that the rebase returned an error (and ideally that it's conflict-related) before asserting IsGitRebaseInProgress.

Copilot · 2026-04-08T11:47:16Z

+	testutil.RunGit(t, dir, "add", "another.txt")
+	testutil.RunGit(t, dir, "commit", "-m", "Another feature commit")


Errors from these git setup commands are ignored. Capture and assert errors from RunGit (add/commit) so the test reliably creates the intended history before running feature finish.

Suggested change

testutil.RunGit(t, dir, "add", "another.txt")

testutil.RunGit(t, dir, "commit", "-m", "Another feature commit")

output, err = testutil.RunGit(t, dir, "add", "another.txt")

if err != nil {

t.Fatalf("Failed to add file another.txt for feature another-feature: %v\nOutput: %s", err, output)

}

output, err = testutil.RunGit(t, dir, "commit", "-m", "Another feature commit")

if err != nil {

t.Fatalf("Failed to commit file another.txt for feature another-feature: %v\nOutput: %s", err, output)

}

Copilot · 2026-04-08T11:47:16Z

 			BranchType:     "feature",
 			BranchName:     "test-feature",
-			CurrentStep:    "merge",
-			ParentBranch:   "develop",
+			CurrentStep:    "delete_branch",
+			ParentBranch:   "main",
 			MergeStrategy:  "merge",
-			FullBranchName: "feature/test-feature",
+			FullBranchName: "worktree-branch",


In this test state, BranchName (test-feature) does not correspond to FullBranchName (worktree-branch). Since BranchName is intended to be the (short) name of the branch being finished and FullBranchName the full ref, keeping them consistent will make the test data closer to real merge state and reduce brittleness if validation expands later.

Copilot · 2026-04-08T11:47:16Z

-			BranchName: "worktree1-feature",
+			Action:         "finish",
+			BranchType:     "feature",
+			BranchName:     "worktree1-feature",


In this test state, BranchName (worktree1-feature) does not correspond to FullBranchName (worktree1-branch). Consider making these fields consistent (short vs full branch name) so the saved state mirrors real-world finish state more closely.

Suggested change

BranchName: "worktree1-feature",

BranchName: "worktree1-branch",

Copilot · 2026-04-08T11:47:16Z

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


Errors from these git setup commands are ignored (add/commit). If any of them fails, the test may not actually create the intended conflicting history. Capture and assert errors from RunGit calls so the conflict setup is reliable.

Suggested change

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.RunGit(t, dir, "add", "conflict.txt")

if err != nil {

t.Fatalf("Failed to add feature conflict file: %v\nOutput: %s", err, output)

}

output, err = testutil.RunGit(t, dir, "commit", "-m", "Feature commit")

if err != nil {

t.Fatalf("Failed to commit feature conflict file: %v\nOutput: %s", err, output)

}

// Add conflicting content on develop

output, err = testutil.RunGit(t, dir, "checkout", "develop")

if err != nil {

t.Fatalf("Failed to checkout develop branch: %v\nOutput: %s", err, output)

}

testutil.WriteFile(t, dir, "conflict.txt", "develop content")

output, err = testutil.RunGit(t, dir, "add", "conflict.txt")

if err != nil {

t.Fatalf("Failed to add develop conflict file: %v\nOutput: %s", err, output)

}

output, err = testutil.RunGit(t, dir, "commit", "-m", "Develop commit")

if err != nil {

t.Fatalf("Failed to commit develop conflict file: %v\nOutput: %s", err, output)

}

// Switch back and finish — will produce conflict

output, err = testutil.RunGit(t, dir, "checkout", "feature/conflict-test")

if err != nil {

t.Fatalf("Failed to checkout feature/conflict-test branch: %v\nOutput: %s", err, output)

}

Copilot · 2026-04-08T11:47:17Z

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


Errors from these git setup commands are ignored (checkout/add/commit). Capture and assert errors from RunGit calls so the conflict setup is deterministic before invoking feature finish.

Suggested change

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.RunGit(t, dir, "add", "conflict.txt")

if err != nil {

t.Fatalf("Failed to add conflict.txt on feature/conflict-test: %v\nOutput: %s", err, output)

}

output, err = testutil.RunGit(t, dir, "commit", "-m", "Feature commit")

if err != nil {

t.Fatalf("Failed to commit feature changes on feature/conflict-test: %v\nOutput: %s", err, output)

}

// Add conflicting content on develop

output, err = testutil.RunGit(t, dir, "checkout", "develop")

if err != nil {

t.Fatalf("Failed to checkout branch develop: %v\nOutput: %s", err, output)

}

testutil.WriteFile(t, dir, "conflict.txt", "develop content")

output, err = testutil.RunGit(t, dir, "add", "conflict.txt")

if err != nil {

t.Fatalf("Failed to add conflict.txt on develop: %v\nOutput: %s", err, output)

}

output, err = testutil.RunGit(t, dir, "commit", "-m", "Develop commit")

if err != nil {

t.Fatalf("Failed to commit conflicting changes on develop: %v\nOutput: %s", err, output)

}

// Switch back and finish — will produce conflict

output, err = testutil.RunGit(t, dir, "checkout", "feature/conflict-test")

if err != nil {

t.Fatalf("Failed to checkout branch feature/conflict-test: %v\nOutput: %s", err, output)

}

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 <noreply@anthropic.com>

alexrinass · 2026-04-08T12:43:24Z

Addressed review feedback in f8bb672.

Accepted (8 comments):

Clear corrupted merge state on load error — IsMergeInProgress() now attempts ClearMergeState() when LoadMergeState() returns an error
Handle ClearMergeState error — note is only printed when clear succeeds
GitFlowMergeStateExists fails on error — uses t.Fatalf instead of silent return false
Error checks in test setup (5 comments) — all RunGit add/commit/checkout calls in the new stale-state tests now check errors

Dismissed (5 comments):

#4-6 (assert merge/rebase command errors): Tests are self-verifying — deterministic conflict setup means if the merge unexpectedly succeeds, the state-detection assertion itself fails
Add publish command for topic branches #10-11 (BranchName↔FullBranchName consistency in worktree tests): Validation only checks FullBranchName for branch existence; BranchName is display-only and the relationship isn't validated

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Copilot · 2026-04-08T12:50:26Z

+
+	// Start merge that will conflict
+	_, _ = testutil.RunGit(t, dir, "merge", "feature")
+
+	withGitRepo(t, dir, func() {


This test intentionally triggers a merge conflict, but it discards the error/output from git merge. Please assert that the merge command failed due to a conflict (e.g., err != nil and output indicates a conflict) so the test fails with a clear reason if the setup does not actually enter a merge state.

Copilot · 2026-04-08T12:50:27Z

+	// Start squash merge that will conflict
+	_, _ = testutil.RunGit(t, dir, "merge", "--squash", "feature")
+
+	withGitRepo(t, dir, func() {


This test intentionally triggers a squash-merge conflict, but it discards the error/output from git merge --squash. Please assert that the command failed due to a conflict so the test reliably verifies the squash-merge detection logic.

Copilot · 2026-04-08T12:50:27Z

+	if !isStateValid(state) {
+		if err := ClearMergeState(); err == nil {
+			fmt.Fprintf(os.Stderr, "Note: Cleared stale merge state from a previous operation\n")
+		}


ClearMergeState() errors are also silently ignored here. If removal fails (e.g., permissions), users may keep hitting the same stale-state behavior without understanding why. Consider emitting a warning to stderr when ClearMergeState returns an error in this path too.

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 <noreply@anthropic.com>

alexrinass · 2026-04-08T13:26:10Z

Addressed round 2 review in b338505.

Accepted (1 comment):

Warn when ClearMergeState fails — both error paths now emit Warning: Failed to clear stale merge state: <error> to stderr instead of silently ignoring the failure

Dismissed (2 comments):

Assert merge/squash command errors in unit tests — same concern from round 1, already dismissed: tests are self-verifying since the state-detection assertion fails if the conflict setup doesn't produce the expected state

alexrinass requested a review from Copilot April 8, 2026 11:40

Copilot started reviewing on behalf of alexrinass April 8, 2026 11:41 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

alexrinass requested a review from Copilot April 8, 2026 12:44

Copilot started reviewing on behalf of alexrinass April 8, 2026 12:44 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

		fmt.Fprintf(os.Stderr, "Note: Cleared stale merge state from a previous operation\n")
		ClearMergeState()

	return false
	t.Fatalf("Failed to determine git directory for repo %s: %v", dir, err)

		// Start merge that will conflict
		_, _ = testutil.RunGit(t, dir, "merge", "feature")

		testutil.RunGit(t, dir, "add", "another.txt")
		testutil.RunGit(t, dir, "commit", "-m", "Another feature commit")

	BranchName: "worktree1-feature",
	BranchName: "worktree1-branch",

Conversation

alexrinass commented Apr 8, 2026

Remarks

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

alexrinass commented Apr 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

alexrinass commented Apr 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants