Skip to content

Commit f1f99a5

Browse files
authored
[feat] merge auto-cleanup deletes the merged remote branch (#277) (#305)
* [feat] merge auto-cleanup deletes the merged remote branch (#277) After a successful merge-to-main, rimba merge now deletes the remote branch (origin/<branch>) in the same best-effort step that clean --merged already performs, reaching behavioural parity with #231. Remote deletion is skipped when no origin is configured, suppressed by --keep/--delete, previewed in --dry-run, and never fails the merge itself (captured in RemoteError, surfaced in CLI output and MCP remote_deleted). * [fix] address PR #305 review feedback - Surface RemoteError in failure output (%v) so users see the deletion reason - Document wtRemoved-only gate: defers remote cleanup on partial failure to rimba clean - Replace gitSubcmdStashPush with gitCmdPush in cmd/merge_test.go; add explicit remote arm - Assert --delete flag in TestMergeWorktreeRemoteDeleteSuccess push args - Remove dead wtDir / resolver.WorktreePath lines from both new e2e tests
1 parent 7a3706f commit f1f99a5

8 files changed

Lines changed: 404 additions & 5 deletions

File tree

cmd/merge.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,13 @@ var mergeCmd = &cobra.Command{
107107
fmt.Fprintf(cmd.OutOrStdout(), "Merged successfully but failed to remove worktree: %v\nTo remove manually: rimba remove %s\n", result.RemoveError, sourceTask)
108108
}
109109

110+
if result.RemoteDeleted {
111+
fmt.Fprintf(cmd.OutOrStdout(), "Deleted remote branch: %s/%s\n", git.DefaultRemote, result.SourceBranch)
112+
} else if result.RemoteError != nil {
113+
fmt.Fprintf(cmd.OutOrStdout(), "Failed to delete remote branch %s/%s: %v\nTo delete remote: git push %s --delete %s\n",
114+
git.DefaultRemote, result.SourceBranch, result.RemoteError, git.DefaultRemote, result.SourceBranch)
115+
}
116+
110117
return nil
111118
},
112119
}

cmd/merge_test.go

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ import (
1010
"github.com/lugassawan/rimba/internal/config"
1111
)
1212

13-
const mergeWorktreeOut = "worktree /repo\nHEAD abc123\nbranch refs/heads/main\n\nworktree /wt/feature-login\nHEAD def456\nbranch refs/heads/feature/login\n\nworktree /wt/feature-dashboard\nHEAD ghi789\nbranch refs/heads/feature/dashboard\n"
13+
const (
14+
mergeWorktreeOut = "worktree /repo\nHEAD abc123\nbranch refs/heads/main\n\nworktree /wt/feature-login\nHEAD def456\nbranch refs/heads/feature/login\n\nworktree /wt/feature-dashboard\nHEAD ghi789\nbranch refs/heads/feature/dashboard\n"
15+
gitCmdPush = "push" // first arg of "git push <remote> ..." — distinct from gitSubcmdStashPush which is args[1] of "git stash push"
16+
)
1417

1518
func mergeTestRunner(mergeErr error) *mockRunner {
1619
return &mockRunner{
@@ -479,3 +482,76 @@ func TestMergeWithNoFF(t *testing.T) {
479482
t.Errorf("merge args = %v, want --no-ff to be present", mergeArgs)
480483
}
481484
}
485+
486+
func TestMergeRemoteDeletedOutput(t *testing.T) {
487+
cfg := &config.Config{DefaultSource: branchMain, WorktreeDir: defaultRelativeWtDir}
488+
// mergeTestRunner returns ("", nil) for unmatched run calls → origin present, push succeeds.
489+
r := mergeTestRunner(nil)
490+
restore := overrideNewRunner(r)
491+
defer restore()
492+
493+
cmd, buf := newTestCmd()
494+
cmd.Flags().String(flagInto, "", "")
495+
cmd.Flags().Bool(flagNoFF, false, "")
496+
cmd.Flags().Bool(flagKeep, false, "")
497+
cmd.Flags().Bool(flagDelete, false, "")
498+
cmd.SetContext(config.WithConfig(context.Background(), cfg))
499+
500+
if err := mergeCmd.RunE(cmd, []string{"login"}); err != nil {
501+
t.Fatalf(fatalMergeRunE, err)
502+
}
503+
out := buf.String()
504+
if !strings.Contains(out, "Deleted remote branch: origin/feature/login") {
505+
t.Errorf("output = %q, want 'Deleted remote branch: origin/feature/login'", out)
506+
}
507+
}
508+
509+
func TestMergeRemoteDeleteFailedOutput(t *testing.T) {
510+
cfg := &config.Config{DefaultSource: branchMain, WorktreeDir: defaultRelativeWtDir}
511+
r := &mockRunner{
512+
run: func(args ...string) (string, error) {
513+
if len(args) >= 2 && args[1] == cmdShowToplevel {
514+
return repoPath, nil
515+
}
516+
if len(args) >= 1 && args[0] == "remote" {
517+
return "https://github.com/lugassawan/rimba.git", nil
518+
}
519+
if len(args) >= 1 && args[0] == gitCmdPush {
520+
return "", errors.New("connection refused")
521+
}
522+
return mergeWorktreeOut, nil
523+
},
524+
runInDir: func(_ string, args ...string) (string, error) {
525+
if len(args) >= 1 && args[0] == cmdStatus {
526+
return "", nil
527+
}
528+
if len(args) >= 1 && args[0] == flagSyncMerge {
529+
return "", nil
530+
}
531+
return "", nil
532+
},
533+
}
534+
restore := overrideNewRunner(r)
535+
defer restore()
536+
537+
cmd, buf := newTestCmd()
538+
cmd.Flags().String(flagInto, "", "")
539+
cmd.Flags().Bool(flagNoFF, false, "")
540+
cmd.Flags().Bool(flagKeep, false, "")
541+
cmd.Flags().Bool(flagDelete, false, "")
542+
cmd.SetContext(config.WithConfig(context.Background(), cfg))
543+
544+
if err := mergeCmd.RunE(cmd, []string{"login"}); err != nil {
545+
t.Fatalf(fatalMergeRunE, err)
546+
}
547+
out := buf.String()
548+
if !strings.Contains(out, "Failed to delete remote branch") {
549+
t.Errorf("output = %q, want 'Failed to delete remote branch'", out)
550+
}
551+
if !strings.Contains(out, "connection refused") {
552+
t.Errorf("output = %q, want remote error reason 'connection refused'", out)
553+
}
554+
if !strings.Contains(out, "git push origin --delete") {
555+
t.Errorf("output = %q, want manual hint with 'git push origin --delete'", out)
556+
}
557+
}

internal/mcp/tool_merge.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ func handleMerge(hctx *HandlerContext) server.ToolHandlerFunc {
7070
Source: result.SourceBranch,
7171
Into: result.TargetLabel,
7272
SourceRemoved: result.SourceRemoved,
73+
RemoteDeleted: result.RemoteDeleted,
7374
})
7475
}
7576
}

internal/mcp/types.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ type mergeResult struct {
6161
Source string `json:"source"`
6262
Into string `json:"into"`
6363
SourceRemoved bool `json:"source_removed"`
64+
RemoteDeleted bool `json:"remote_deleted,omitempty"`
6465
}
6566

6667
// syncResult holds the outcome of a sync operation.

internal/operations/merge.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ type MergeResult struct {
3434
SourceRemoved bool // true only if both worktree removed and branch deleted
3535
WorktreeRemoved bool // true if worktree was removed (branch may still exist)
3636
RemoveError error // non-nil if cleanup failed
37+
RemoteDeleted bool // true if the remote branch was deleted (parity with clean --merged, #231)
38+
RemoteError error // non-nil if remote-branch deletion was attempted and failed
3739
Plan *Plan // always non-nil on a successful return; records steps that were (or would be) executed
3840
}
3941

@@ -121,11 +123,36 @@ func MergeWorktree(ctx context.Context, r git.Runner, params MergeParams, onProg
121123
if rmErr != nil {
122124
result.RemoveError = rmErr
123125
}
126+
127+
// Delete the merged remote branch (best-effort, parity with clean --merged, #231).
128+
// Gated on wtRemoved (not brDeleted) so that a partial failure — branch deleted but
129+
// worktree still on disk — defers remote cleanup to a later `rimba clean --merged` run.
130+
deleteMergedRemote(ctx, r, plan, source.Branch, &result, params.DryRun, wtRemoved)
124131
}
125132

126133
return result, nil
127134
}
128135

136+
// deleteMergedRemote deletes the remote tracking branch after a merge cleanup.
137+
// It is a no-op when the remote is absent or when the worktree was not actually
138+
// removed. Failures are captured in result.RemoteError and never abort the merge.
139+
func deleteMergedRemote(ctx context.Context, r git.Runner, plan *Plan, branch string, result *MergeResult, dryRun, wtRemoved bool) {
140+
if !dryRun && !wtRemoved {
141+
return
142+
}
143+
if !git.RemoteExists(ctx, r, git.DefaultRemote) {
144+
return
145+
}
146+
_ = plan.Do("delete remote branch: "+git.DefaultRemote+"/"+branch, func() error {
147+
var remoteErr error
148+
if remoteErr = git.DeleteRemoteBranch(ctx, r, git.DefaultRemote, branch); remoteErr == nil {
149+
result.RemoteDeleted = true
150+
}
151+
result.RemoteError = remoteErr
152+
return nil // best-effort: never abort the merge
153+
})
154+
}
155+
129156
// abortFailedMerge attempts to roll back a failed merge in targetDir.
130157
// It checks MERGE_HEAD first so we only abort when a merge is actually in progress.
131158
// If the abort also fails, both errors are surfaced with a manual-cleanup hint.

0 commit comments

Comments
 (0)