Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 27 additions & 16 deletions internal/cli/clean.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,28 @@ func init() {
cleanCmd.Flags().BoolVarP(&cleanDry, "dry-run", "n", false, "Show what would be removed without removing")
}

// selectMergedWorktrees returns the worktrees eligible for cleanup: non-primary
// worktrees whose branch is in the merged set. The default branch is never a
// candidate (MergedBranches no longer self-filters it now that it measures
// against a fully-qualified ref).
func selectMergedWorktrees(worktrees []*worktree.Worktree, mergedBranches []string, defaultBranch string) []*worktree.Worktree {
mergedSet := make(map[string]bool, len(mergedBranches))
for _, b := range mergedBranches {
mergedSet[b] = true
}

var toRemove []*worktree.Worktree
for _, wt := range worktrees {
if wt.IsPrimary || wt.Branch == defaultBranch {
continue
}
if mergedSet[wt.Branch] {
toRemove = append(toRemove, wt)
}
}
return toRemove
}

func runClean(cmd *cobra.Command, args []string) error {
cwd, err := os.Getwd()
if err != nil {
Expand All @@ -57,27 +79,16 @@ func runClean(cmd *cobra.Command, args []string) error {
return err
}

// Find merged branches
mergedBranches, err := wm.MergedBranches(defaultBranch)
// Find merged branches, measured against the resolved default tip
// (origin/<default> when local is stale) so branches merged upstream are
// detected consistently with how `ygg new` bases new worktrees.
mergedBranches, err := wm.MergedBranches(wm.BaseRef(defaultBranch))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep the default branch out of clean candidates

When BaseRef returns a fully-qualified ref here, MergedBranches no longer filters out the default branch itself: for example, running ygg clean from a non-primary worktree checked out on main makes git branch --merged refs/heads/main print * main, which MergedBranches normalizes to main and compares against refs/heads/main. That puts the default branch in mergedSet, so the current default-branch worktree is scheduled for removal; the previous MergedBranches(defaultBranch) call filtered main out.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 61a7d2c. You're right — once merge detection uses a fully-qualified ref, MergedBranches no longer self-filters the default branch. Extracted the candidate selection into selectMergedWorktrees and now skip both the primary worktree and any worktree on the default branch, independent of the merged set. Added TestSelectMergedWorktreesExcludesDefaultBranch (which fails on the old logic, selecting the main worktree, and passes now). IsBranchMerged was unaffected (it returns early for branch == defaultBranch).

if err != nil {
errorMsg("Failed to get merged branches: %v", err)
return err
}

mergedSet := make(map[string]bool)
for _, b := range mergedBranches {
mergedSet[b] = true
}

var toRemove []*worktree.Worktree
for _, wt := range worktrees {
if wt.IsPrimary {
continue
}
if mergedSet[wt.Branch] {
toRemove = append(toRemove, wt)
}
}
toRemove := selectMergedWorktrees(worktrees, mergedBranches, defaultBranch)

if len(toRemove) == 0 {
info("No merged worktrees to clean up")
Expand Down
31 changes: 31 additions & 0 deletions internal/cli/clean_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package cli

import (
"testing"

"github.com/joch/ygg/internal/worktree"
)

func TestSelectMergedWorktreesExcludesDefaultBranch(t *testing.T) {
worktrees := []*worktree.Worktree{
{Name: "main", Branch: "main", IsPrimary: true},
{Name: "mainwt", Branch: "main", IsPrimary: false}, // non-primary, on default branch
{Name: "feature", Branch: "feature", IsPrimary: false},
{Name: "wip", Branch: "wip", IsPrimary: false},
}
// MergedBranches now measures against a fully-qualified ref, so the default
// branch itself appears in the merged list (it no longer self-filters).
merged := []string{"main", "feature"}

got := selectMergedWorktrees(worktrees, merged, "main")

// Only "feature" qualifies: primary is skipped, the non-primary default-branch
// worktree must be skipped, and "wip" is not merged.
var names []string
for _, wt := range got {
names = append(names, wt.Name)
}
if len(names) != 1 || names[0] != "feature" {
t.Fatalf("selected %v, want exactly [feature]", names)
}
}
17 changes: 12 additions & 5 deletions internal/cli/new.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@ var newCmd = &cobra.Command{
Long: `Create a new git worktree with the specified name.

This will:
1. Fetch and pull the default branch (main/master)
2. Create a new worktree with a branch named <name>
1. Fetch the latest changes from origin
2. Create a new worktree with a branch named <name>, based on the latest
origin/<default-branch> (falling back to the local default branch when
there is no remote)
3. Enter a subshell in the new worktree directory

Exit the subshell to return to your original directory.`,
Expand Down Expand Up @@ -50,22 +52,27 @@ func runNew(cmd *cobra.Command, args []string) error {
info("Could not fetch (offline?)")
}

// Pull default branch in main repo
// Detect the default branch (main/master); the worktree is based on the
// freshly fetched origin/<default> tip, so no local pull is required.
defaultBranch, err := wm.DefaultBranch()
if err != nil {
errorMsg("Could not detect default branch: %v", err)
return err
}

info("Creating worktree: %s (based on %s)", name, defaultBranch)
info("Creating worktree: %s (default branch %s)", name, defaultBranch)

wt, err := wm.Create(name)
if err != nil {
errorMsg("Failed to create worktree: %v", err)
return err
}

success("Created worktree at %s", wt.Path)
if wt.Base != "" {
success("Created worktree at %s (based on %s)", wt.Path, wt.Base)
} else {
success("Created worktree at %s", wt.Path)
}

// Warn if .worktrees is not in .gitignore
if !wm.IsWorktreeDirIgnored() {
Expand Down
106 changes: 80 additions & 26 deletions internal/worktree/worktree.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ type Worktree struct {
Name string
Path string
Branch string
Base string // Ref the new branch was based on (empty when checking out an existing branch)
IsBare bool
IsLocked bool
IsPrimary bool
Expand Down Expand Up @@ -90,6 +91,65 @@ func (m *Manager) DefaultBranch() (string, error) {
return "", fmt.Errorf("could not detect default branch")
}

// BaseRef returns the effective tip of the default branch — the ref new
// worktrees are based on and that merge detection is measured against. It
// prefers the freshly fetched origin/<defaultBranch> remote-tracking ref so it
// reflects the latest upstream even when the local default branch is stale
// (the common "forgot to pull" case) — but only when origin is ahead of, or
// equal to, the local branch. When the local branch has commits origin lacks
// (committed directly to it, or fetch failed offline), or the two have
// diverged, the local branch wins so those commits are never silently dropped.
// Falls back to the local branch when no remote-tracking ref exists.
//
// The returned ref is fully qualified (refs/remotes/... or refs/heads/...) so it
// stays unambiguous when passed to git even in repos that have a local branch
// named e.g. "origin/main". Use DisplayRef for user-facing output.
func (m *Manager) BaseRef(defaultBranch string) string {
remoteRef := "refs/remotes/origin/" + defaultBranch
localRef := "refs/heads/" + defaultBranch
hasRemote := m.refExists(remoteRef)
hasLocal := m.refExists(localRef)

switch {
case hasRemote && hasLocal:
// Use origin only when the local branch is an ancestor of it
// (origin is ahead or identical); otherwise keep local.
if m.isAncestor(localRef, remoteRef) {
return remoteRef
}
return localRef
case hasRemote:
return remoteRef
default:
return localRef
}
}

// DisplayRef shortens a fully-qualified ref for user-facing output, e.g.
// refs/remotes/origin/main -> origin/main and refs/heads/main -> main.
func DisplayRef(ref string) string {
for _, prefix := range []string{"refs/remotes/", "refs/heads/"} {
if s := strings.TrimPrefix(ref, prefix); s != ref {
return s
}
}
return ref
}

// refExists reports whether the given fully-qualified ref resolves in the repo.
func (m *Manager) refExists(ref string) bool {
cmd := exec.Command("git", "rev-parse", "--verify", "--quiet", ref)
cmd.Dir = m.repoPath
return cmd.Run() == nil
}

// isAncestor reports whether ref a is an ancestor of (or identical to) ref b.
func (m *Manager) isAncestor(a, b string) bool {
cmd := exec.Command("git", "merge-base", "--is-ancestor", a, b)
cmd.Dir = m.repoPath
return cmd.Run() == nil
}

// Fetch fetches from origin.
func (m *Manager) Fetch() error {
cmd := exec.Command("git", "fetch", "origin")
Expand All @@ -101,17 +161,6 @@ func (m *Manager) Fetch() error {
return nil
}

// Pull pulls the current branch from origin.
func (m *Manager) Pull() error {
cmd := exec.Command("git", "pull")
cmd.Dir = m.repoPath
output, err := cmd.CombinedOutput()
if err != nil {
return fmt.Errorf("git pull failed: %w\n%s", err, output)
}
return nil
}

// HasUncommittedChanges returns true if there are uncommitted changes.
func (m *Manager) HasUncommittedChanges(path string) bool {
cmd := exec.Command("git", "status", "--porcelain")
Expand Down Expand Up @@ -140,22 +189,24 @@ func (m *Manager) Create(name string) (*Worktree, error) {
return nil, fmt.Errorf("failed to detect default branch: %w", err)
}

// Create worktree with new branch based on default branch
cmd := exec.Command("git", "worktree", "add", "-b", name, worktreePath, defaultBranch)
var base string
var cmd *exec.Cmd
if m.refExists("refs/heads/" + name) {
// A branch with this name already exists — check it out into the new
// worktree as-is, without re-pointing it at a base ref.
cmd = exec.Command("git", "worktree", "add", worktreePath, name)
} else {
// Base the new branch on the freshly fetched origin/<default> tip when
// it is ahead of the local branch (the "forgot to pull" case),
// otherwise on the local default branch — see BaseRef. --no-track keeps
// the new branch from adopting origin/<default> as its upstream.
base = m.BaseRef(defaultBranch)
cmd = exec.Command("git", "worktree", "add", "--no-track", "-b", name, worktreePath, base)
}
cmd.Dir = m.repoPath

output, err := cmd.CombinedOutput()
if err != nil {
// Branch might already exist, try without -b
if strings.Contains(string(output), "already exists") {
cmd = exec.Command("git", "worktree", "add", worktreePath, name)
cmd.Dir = m.repoPath
output, err = cmd.CombinedOutput()
}

if err != nil {
return nil, fmt.Errorf("failed to create worktree: %w\n%s", err, output)
}
if output, err := cmd.CombinedOutput(); err != nil {
return nil, fmt.Errorf("failed to create worktree: %w\n%s", err, output)
}

// Copy untracked/ignored files from main worktree
Expand All @@ -165,6 +216,7 @@ func (m *Manager) Create(name string) (*Worktree, error) {
Name: name,
Path: worktreePath,
Branch: name,
Base: DisplayRef(base),
CopiedFiles: copied,
CopyError: copyErr,
}, nil
Expand Down Expand Up @@ -261,7 +313,9 @@ func (m *Manager) IsBranchMerged(branch string) (bool, error) {
return true, nil
}

merged, err := m.MergedBranches(defaultBranch)
// Measure against the resolved default tip (origin/<default> when local is
// stale) so a branch merged upstream is recognized even before a local pull.
merged, err := m.MergedBranches(m.BaseRef(defaultBranch))
if err != nil {
return false, err
}
Expand Down
Loading
Loading