Skip to content

Commit 5e50fe7

Browse files
authored
[refactor] Extract shared service layer between CLI and MCP (#83)
* [refactor] Add operations foundation: ProgressFunc, deps helpers, ResolveMainBranch Phase 0 of service layer extraction (#70, #81): - Add ProgressFunc type and nil-safe notify helper for operation progress - Move deps install helpers (InstallDeps, InstallDepsPreferSource, WorktreePathsExcluding, RunPostCreateHooks) from cmd to operations - Extract ResolveMainBranch into operations with configDefault parameter * [refactor] Extract Remove operation into shared service layer Phase 1 of service layer extraction (#70, #81): - Add RemoveWorktree operation with RemoveResult struct - Add removeAndCleanup private helper (used by remove, merge, clean) - Convert cmd/remove.go to thin wrapper over operations - Convert internal/mcp/tool_remove.go to thin wrapper over operations * [refactor] Extract clean operations into shared service layer Move FindMergedCandidates, FindStaleCandidates, and RemoveCandidates from inline implementations in cmd/clean.go and mcp/tool_clean.go into internal/operations/clean.go. Both CLI and MCP layers now call the shared operations, keeping only formatting and output logic. Also removes the force parameter from removeAndCleanup since all callers pass false. Phase 2 of #70 / #81 * [refactor] Extract merge operation into shared service layer Move worktree resolution, concurrent dirty checks, merge execution, and auto-cleanup logic from cmd/merge.go and mcp/tool_merge.go into operations.MergeWorktree. Both layers now build MergeParams, call the shared operation, and format the MergeResult for their output format. Also removes checkMergeDirty from MCP (now handled by operations) and fixes the dirtyResult type reference in cmd/merge_bench_test.go. Phase 3 of #70 / #81 * [refactor] Phase 4: Extract Add operation into operations package Move worktree creation logic from cmd/add.go and internal/mcp/tool_add.go into internal/operations/add.go. Both layers are now thin wrappers. Also moves deps/hooks helpers (installDeps, installDepsPreferSource, runHooks, worktreePathsExcluding) into operations/deps_install.go, updating cmd/duplicate.go and cmd/restore.go to use the shared functions. * [refactor] Phase 5: Consolidate resolveMainBranch via operations Remove duplicate resolveMainBranch from internal/mcp/tool_list.go. Both cmd and MCP layers now delegate to operations.ResolveMainBranch. * [test] Add coverage tests for operations package Cover previously untested paths: InstallDeps/InstallDepsPreferSource no-modules early return, RunPostCreateHooks, AddWorktree deps/hooks branches, checkDirty error paths, FindMergedCandidates squash-merge error, FindStaleCandidates commit-time error. Brings total coverage from 96.4% to 97.1%.
1 parent 146bccc commit 5e50fe7

35 files changed

Lines changed: 2048 additions & 1729 deletions

cmd/add.go

Lines changed: 28 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,12 @@ package cmd
22

33
import (
44
"fmt"
5-
"os"
65
"path/filepath"
76

87
"github.com/lugassawan/rimba/internal/config"
9-
"github.com/lugassawan/rimba/internal/deps"
10-
"github.com/lugassawan/rimba/internal/fileutil"
118
"github.com/lugassawan/rimba/internal/git"
129
"github.com/lugassawan/rimba/internal/hint"
13-
"github.com/lugassawan/rimba/internal/resolver"
10+
"github.com/lugassawan/rimba/internal/operations"
1411
"github.com/lugassawan/rimba/internal/spinner"
1512
"github.com/spf13/cobra"
1613
)
@@ -55,22 +52,8 @@ var addCmd = &cobra.Command{
5552
source = cfg.DefaultSource
5653
}
5754

58-
branch := resolver.BranchName(prefix, task)
59-
wtDir := filepath.Join(repoRoot, cfg.WorktreeDir)
60-
wtPath := resolver.WorktreePath(wtDir, branch)
61-
62-
wtEntries, err := git.ListWorktrees(r)
63-
if err != nil {
64-
return err
65-
}
66-
67-
// Validate
68-
if git.BranchExists(r, branch) {
69-
return fmt.Errorf("branch %q already exists", branch)
70-
}
71-
if _, err := os.Stat(wtPath); err == nil {
72-
return fmt.Errorf("worktree path already exists: %s", wtPath)
73-
}
55+
skipDeps, _ := cmd.Flags().GetBool(flagSkipDeps)
56+
skipHooks, _ := cmd.Flags().GetBool(flagSkipHooks)
7457

7558
hint.New(cmd, hintPainter(cmd)).
7659
Add(flagSkipDeps, hintSkipDeps).
@@ -81,54 +64,41 @@ var addCmd = &cobra.Command{
8164
s := spinner.New(spinnerOpts(cmd))
8265
defer s.Stop()
8366

84-
// Create worktree
85-
s.Start("Creating worktree...")
86-
if err := git.AddWorktree(r, wtPath, branch, source); err != nil {
87-
return err
67+
var configModules []config.ModuleConfig
68+
if cfg.Deps != nil {
69+
configModules = cfg.Deps.Modules
8870
}
8971

90-
// Copy files
91-
s.Update("Copying files...")
92-
copied, err := fileutil.CopyEntries(repoRoot, wtPath, cfg.CopyFiles)
72+
result, err := operations.AddWorktree(r, operations.AddParams{
73+
Task: task,
74+
Prefix: prefix,
75+
Source: source,
76+
RepoRoot: repoRoot,
77+
WorktreeDir: filepath.Join(repoRoot, cfg.WorktreeDir),
78+
CopyFiles: cfg.CopyFiles,
79+
SkipDeps: skipDeps,
80+
AutoDetect: cfg.IsAutoDetectDeps(),
81+
ConfigModules: configModules,
82+
SkipHooks: skipHooks,
83+
PostCreate: cfg.PostCreate,
84+
}, func(msg string) { s.Start(msg) })
9385
if err != nil {
94-
return fmt.Errorf("worktree created but failed to copy files: %w\nTo retry, manually copy files to: %s\nTo remove the worktree: rimba remove %s", err, wtPath, task)
95-
}
96-
97-
// Dependencies
98-
skipDeps, _ := cmd.Flags().GetBool(flagSkipDeps)
99-
var depsResults []deps.InstallResult
100-
if !skipDeps {
101-
s.Update("Installing dependencies...")
102-
depsResults = installDeps(r, cfg, wtPath, wtEntries, func(cur, total int, name string) {
103-
s.Update(fmt.Sprintf("Installing dependencies... (%s) [%d/%d]", name, cur, total))
104-
})
105-
}
106-
107-
// Post-create hooks
108-
skipHooks, _ := cmd.Flags().GetBool(flagSkipHooks)
109-
var hookResults []deps.HookResult
110-
if !skipHooks && len(cfg.PostCreate) > 0 {
111-
s.Update("Running hooks...")
112-
hookResults = runHooks(wtPath, cfg.PostCreate, func(cur, total int, name string) {
113-
s.Update(fmt.Sprintf("Running hooks... (%s) [%d/%d]", name, cur, total))
114-
})
86+
return err
11587
}
11688

117-
s.Stop()
118-
11989
out := cmd.OutOrStdout()
12090
fmt.Fprintf(out, "Created worktree for task %q\n", task)
121-
fmt.Fprintf(out, " Branch: %s\n", branch)
122-
fmt.Fprintf(out, " Path: %s\n", wtPath)
123-
if len(copied) > 0 {
124-
fmt.Fprintf(out, " Copied: %v\n", copied)
91+
fmt.Fprintf(out, " Branch: %s\n", result.Branch)
92+
fmt.Fprintf(out, " Path: %s\n", result.Path)
93+
if len(result.Copied) > 0 {
94+
fmt.Fprintf(out, " Copied: %v\n", result.Copied)
12595
}
126-
if skipped := fileutil.SkippedEntries(cfg.CopyFiles, copied); len(skipped) > 0 {
127-
fmt.Fprintf(out, " Skipped (not found): %v\n", skipped)
96+
if len(result.Skipped) > 0 {
97+
fmt.Fprintf(out, " Skipped (not found): %v\n", result.Skipped)
12898
}
12999

130-
printInstallResults(out, depsResults)
131-
printHookResultsList(out, hookResults)
100+
printInstallResults(out, result.DepsResults)
101+
printHookResultsList(out, result.HookResults)
132102

133103
return nil
134104
},

cmd/clean.go

Lines changed: 57 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ import (
44
"bufio"
55
"fmt"
66
"strings"
7-
"time"
87

98
"github.com/lugassawan/rimba/internal/git"
109
"github.com/lugassawan/rimba/internal/hint"
10+
"github.com/lugassawan/rimba/internal/operations"
1111
"github.com/lugassawan/rimba/internal/resolver"
1212
"github.com/lugassawan/rimba/internal/spinner"
1313
"github.com/spf13/cobra"
@@ -19,23 +19,12 @@ const (
1919
flagStale = "stale"
2020

2121
hintMerged = "Remove worktrees whose branches are already merged into main"
22-
hintStale = "Remove worktrees with no recent commits (configure with --stale-days)"
2322
hintDryRunPrune = "Preview what would be pruned without making changes"
2423
hintDryRunMerged = "Preview what would be removed without making changes"
2524
hintDryRunStale = "Preview what would be removed without making changes"
2625
hintForce = "Skip confirmation prompt"
2726
)
2827

29-
type cleanCandidate struct {
30-
path string
31-
branch string
32-
}
33-
34-
type staleCandidate struct {
35-
cleanCandidate
36-
lastCommit time.Time
37-
}
38-
3928
func init() {
4029
cleanCmd.Flags().Bool(flagDryRun, false, "Show what would be pruned/removed without making changes")
4130
cleanCmd.Flags().Bool(flagMerged, false, "Remove worktrees whose branches are merged into main")
@@ -127,7 +116,7 @@ func cleanMerged(cmd *cobra.Command, r git.Runner) error {
127116
}
128117

129118
s.Update("Analyzing branches...")
130-
candidates, err := findMergedCandidates(r, mergeRef, mainBranch)
119+
candidates, err := operations.FindMergedCandidates(r, mergeRef, mainBranch)
131120
if err != nil {
132121
return err
133122
}
@@ -150,82 +139,16 @@ func cleanMerged(cmd *cobra.Command, r git.Runner) error {
150139
return nil
151140
}
152141

153-
removed := removeWorktrees(cmd, r, candidates)
142+
items := operations.RemoveCandidates(r, candidates, func(msg string) {
143+
s.Update(msg)
144+
})
145+
printCleanedItems(cmd, items)
146+
147+
removed := countRemoved(items)
154148
fmt.Fprintf(cmd.OutOrStdout(), "Cleaned %d merged worktree(s).\n", removed)
155149
return nil
156150
}
157151

158-
func findMergedCandidates(r git.Runner, mergeRef, mainBranch string) ([]cleanCandidate, error) {
159-
mergedList, err := git.MergedBranches(r, mergeRef)
160-
if err != nil {
161-
return nil, fmt.Errorf("failed to list merged branches: %w", err)
162-
}
163-
164-
mergedSet := make(map[string]bool, len(mergedList))
165-
for _, b := range mergedList {
166-
mergedSet[b] = true
167-
}
168-
169-
entries, err := git.ListWorktrees(r)
170-
if err != nil {
171-
return nil, err
172-
}
173-
174-
var candidates []cleanCandidate
175-
for _, e := range git.FilterEntries(entries, mainBranch) {
176-
if mergedSet[e.Branch] {
177-
candidates = append(candidates, cleanCandidate{path: e.Path, branch: e.Branch})
178-
continue
179-
}
180-
181-
// Fallback: squash-merge detection
182-
squashed, err := git.IsSquashMerged(r, mergeRef, e.Branch)
183-
if err != nil {
184-
continue
185-
}
186-
if squashed {
187-
candidates = append(candidates, cleanCandidate{path: e.Path, branch: e.Branch})
188-
}
189-
}
190-
return candidates, nil
191-
}
192-
193-
func printMergedCandidates(cmd *cobra.Command, candidates []cleanCandidate) {
194-
prefixes := resolver.AllPrefixes()
195-
fmt.Fprintln(cmd.OutOrStdout(), "Merged worktrees:")
196-
for _, c := range candidates {
197-
task, _ := resolver.TaskFromBranch(c.branch, prefixes)
198-
fmt.Fprintf(cmd.OutOrStdout(), " %s (%s)\n", task, c.branch)
199-
}
200-
}
201-
202-
func confirmRemoval(cmd *cobra.Command, count int, label string) bool {
203-
fmt.Fprintf(cmd.OutOrStdout(), "\nRemove %d %s worktree(s)? [y/N] ", count, label)
204-
reader := bufio.NewReader(cmd.InOrStdin())
205-
answer, _ := reader.ReadString('\n')
206-
answer = strings.TrimSpace(strings.ToLower(answer))
207-
return answer == "y" || answer == "yes"
208-
}
209-
210-
func removeWorktrees(cmd *cobra.Command, r git.Runner, candidates []cleanCandidate) int {
211-
var removed int
212-
for _, c := range candidates {
213-
if err := git.RemoveWorktree(r, c.path, false); err != nil {
214-
fmt.Fprintf(cmd.OutOrStdout(), "Failed to remove worktree %s: %v\nTo remove manually: rimba remove %s\n", c.branch, err, c.branch)
215-
continue
216-
}
217-
fmt.Fprintf(cmd.OutOrStdout(), "Removed worktree: %s\n", c.path)
218-
219-
if err := git.DeleteBranch(r, c.branch, true); err != nil {
220-
fmt.Fprintf(cmd.OutOrStdout(), "Worktree removed but failed to delete branch: %v\nTo delete manually: git branch -D %s\n", err, c.branch)
221-
continue
222-
}
223-
fmt.Fprintf(cmd.OutOrStdout(), "Deleted branch: %s\n", c.branch)
224-
removed++
225-
}
226-
return removed
227-
}
228-
229152
func cleanStale(cmd *cobra.Command, r git.Runner) error {
230153
dryRun, _ := cmd.Flags().GetBool(flagDryRun)
231154
force, _ := cmd.Flags().GetBool(flagForce)
@@ -246,7 +169,7 @@ func cleanStale(cmd *cobra.Command, r git.Runner) error {
246169
defer s.Stop()
247170

248171
s.Start("Analyzing worktree activity...")
249-
candidates, err := findStaleCandidates(r, mainBranch, staleDays)
172+
candidates, err := operations.FindStaleCandidates(r, mainBranch, staleDays)
250173
if err != nil {
251174
return err
252175
}
@@ -268,47 +191,69 @@ func cleanStale(cmd *cobra.Command, r git.Runner) error {
268191
return nil
269192
}
270193

271-
toRemove := make([]cleanCandidate, len(candidates))
194+
toRemove := make([]operations.CleanCandidate, len(candidates))
272195
for i, c := range candidates {
273-
toRemove[i] = c.cleanCandidate
196+
toRemove[i] = c.CleanCandidate
274197
}
275198

276-
removed := removeWorktrees(cmd, r, toRemove)
199+
items := operations.RemoveCandidates(r, toRemove, func(msg string) {
200+
s.Update(msg)
201+
})
202+
printCleanedItems(cmd, items)
203+
204+
removed := countRemoved(items)
277205
fmt.Fprintf(cmd.OutOrStdout(), "Cleaned %d stale worktree(s).\n", removed)
278206
return nil
279207
}
280208

281-
func findStaleCandidates(r git.Runner, mainBranch string, staleDays int) ([]staleCandidate, error) {
282-
entries, err := git.ListWorktrees(r)
283-
if err != nil {
284-
return nil, err
209+
func printMergedCandidates(cmd *cobra.Command, candidates []operations.CleanCandidate) {
210+
prefixes := resolver.AllPrefixes()
211+
fmt.Fprintln(cmd.OutOrStdout(), "Merged worktrees:")
212+
for _, c := range candidates {
213+
task, _ := resolver.TaskFromBranch(c.Branch, prefixes)
214+
fmt.Fprintf(cmd.OutOrStdout(), " %s (%s)\n", task, c.Branch)
215+
}
216+
}
217+
218+
func printStaleCandidates(cmd *cobra.Command, candidates []operations.StaleCandidate) {
219+
prefixes := resolver.AllPrefixes()
220+
fmt.Fprintln(cmd.OutOrStdout(), "Stale worktrees:")
221+
for _, c := range candidates {
222+
task, _ := resolver.TaskFromBranch(c.Branch, prefixes)
223+
age := resolver.FormatAge(c.LastCommit)
224+
fmt.Fprintf(cmd.OutOrStdout(), " %s (%s) — last commit: %s\n", task, c.Branch, age)
285225
}
226+
}
286227

287-
threshold := time.Now().Add(-time.Duration(staleDays) * 24 * time.Hour)
228+
func confirmRemoval(cmd *cobra.Command, count int, label string) bool {
229+
fmt.Fprintf(cmd.OutOrStdout(), "\nRemove %d %s worktree(s)? [y/N] ", count, label)
230+
reader := bufio.NewReader(cmd.InOrStdin())
231+
answer, _ := reader.ReadString('\n')
232+
answer = strings.TrimSpace(strings.ToLower(answer))
233+
return answer == "y" || answer == "yes"
234+
}
288235

289-
var candidates []staleCandidate
290-
for _, e := range git.FilterEntries(entries, mainBranch) {
291-
ct, err := git.LastCommitTime(r, e.Branch)
292-
if err != nil {
236+
func printCleanedItems(cmd *cobra.Command, items []operations.CleanedItem) {
237+
for _, item := range items {
238+
if !item.WorktreeRemoved {
239+
fmt.Fprintf(cmd.OutOrStdout(), "Failed to remove worktree %s\nTo remove manually: rimba remove %s\n", item.Branch, item.Branch)
293240
continue
294241
}
295-
296-
if ct.Before(threshold) {
297-
candidates = append(candidates, staleCandidate{
298-
cleanCandidate: cleanCandidate{path: e.Path, branch: e.Branch},
299-
lastCommit: ct,
300-
})
242+
fmt.Fprintf(cmd.OutOrStdout(), "Removed worktree: %s\n", item.Path)
243+
if item.BranchDeleted {
244+
fmt.Fprintf(cmd.OutOrStdout(), "Deleted branch: %s\n", item.Branch)
245+
} else {
246+
fmt.Fprintf(cmd.OutOrStdout(), "Worktree removed but failed to delete branch\nTo delete manually: git branch -D %s\n", item.Branch)
301247
}
302248
}
303-
return candidates, nil
304249
}
305250

306-
func printStaleCandidates(cmd *cobra.Command, candidates []staleCandidate) {
307-
prefixes := resolver.AllPrefixes()
308-
fmt.Fprintln(cmd.OutOrStdout(), "Stale worktrees:")
309-
for _, c := range candidates {
310-
task, _ := resolver.TaskFromBranch(c.branch, prefixes)
311-
age := resolver.FormatAge(c.lastCommit)
312-
fmt.Fprintf(cmd.OutOrStdout(), " %s (%s) — last commit: %s\n", task, c.branch, age)
251+
func countRemoved(items []operations.CleanedItem) int {
252+
count := 0
253+
for _, item := range items {
254+
if item.WorktreeRemoved && item.BranchDeleted {
255+
count++
256+
}
313257
}
258+
return count
314259
}

0 commit comments

Comments
 (0)